From: Doug Harrison [MVP] on
On Tue, 27 Apr 2010 19:56:07 -0400, "RB" <NoMail(a)NoSpam> wrote:

>Thanks Doug I see now where the compiler is reading this from.
>Also I have a couple of questions if you would be so kind as to
>comment on.
>Was (or is ) the older strNcpy just as safe as the newer StringCchCopy,
> "if the programmer does proper TotalSize / UnitSize checking for the N " ?
>And when I say "as safe" I am not just talking about making sure the
>programmer doesn't screw up but also malicious buffer overflow code
>taking advantage of this type of code.

It's exactly as safe WRT buffer overruns as long as the code is correct.
However, I don't use the "safe" string functions and don't know offhand if
StringCchCopy is exactly the same as strncpy, which is a very weird
function, one that can leave the destination buffer a non-string, as it
will not be nul-terminated if the source string is too long. What's so
bizarre to me is the idea of recovering from a failed string copy instead
of setting it up so it can't fail in the first place. That's why the
Windows-level functions make no sense to me, and the CRT-level functions
make at least a tiny bit of sense, in that they crash the program by
default. IMO, that should be an immutable property, because people aren't
going to faithfully check return codes.

>( keep in mind I understand buffer overflow but do not understand the
> how it can be used by malicious code insertion )

One way on x86 is for a buffer overrun of a local variable to write a value
into a saved return address on the stack, such that when the function
returns, the CPU does not return to the caller and resume execution, but
instead jumps to some code somehow injected into that location. This is a
problem because people do stupid things that amount to:

void f(const char* string_of_unknown_length)
{
char s[40];
strcpy(s, string_of_unknown_length);
}

The idea behind the "safe" string functions is to make sloppy programmers
think about buffer sizes and string lengths, which hopefully reduces the
frequency of these inexcusable blunders. However, it's still possible to
get the lengths and whatnot wrong when using "safe" string functions, which
quite a few MSDN examples did after the big "security push" in 2003 or so.

--
Doug Harrison
Visual C++ MVP
From: Giovanni Dicanio on
"Doug Harrison [MVP]" <dsh(a)mvps.org> ha scritto nel messaggio
news:83vet5h54uopeq9o1f6ajke1lqo0bd6kvf(a)4ax.com...

> It's exactly as safe WRT buffer overruns as long as the code is correct.
> However, I don't use the "safe" string functions and don't know offhand if
> StringCchCopy is exactly the same as strncpy, which is a very weird
> function, one that can leave the destination buffer a non-string, as it
> will not be nul-terminated if the source string is too long.

Doug: I believe that both StringCchCopy and StringCchCopyN always terminate
the destination buffer with NUL.


> What's so
> bizarre to me is the idea of recovering from a failed string copy instead
> of setting it up so it can't fail in the first place. That's why the
> Windows-level functions make no sense to me, and the CRT-level functions
> make at least a tiny bit of sense, in that they crash the program by
> default. IMO, that should be an immutable property, because people aren't
> going to faithfully check return codes.

I think that the usefulness of the safe string functions is that they always
put a NUL-terminator in the destination buffer (so even if the programmer
forgets to check the return value, at least the destination buffer contains
a well-formed string, yes truncated, but at least NUL-terminated).

Moreover, I think these functions can be useful when you use them at the
boundary of a pure-C interface library.
I mean, suppose we have a library that accepts a string.
We have no control to the string passed by the caller (from outside our
library), so if we have to copy this string internally, the StringCchCopy
function can come in handy:

HRESULT DoSomething(const WCHAR * someInputText)
{
WCHAR workingBuffer[ 1024 ];
HRESULT hr;
...

// Don't trust input string coming from the outside!
hr = StringCchCopy( workingBuffer, _countof(workingBuffer),
someInputText );
if (FAILED(hr))
return hr;

...
}

If you want to use the CRT functions, you should call first e.g. wcslen,
then if the length of the input string is in proper range, you can call
wcscpy.
All in all, the safe string functions seem more convenient for this kind of
job (IMHO).


> The idea behind the "safe" string functions is to make sloppy programmers
> think about buffer sizes and string lengths, which hopefully reduces the
> frequency of these inexcusable blunders. However, it's still possible to
> get the lengths and whatnot wrong when using "safe" string functions,

I do agree with you on this point.

I think a common mistake is to use StringCchCopy with sizeof() instead of
_countof() for the size of the destination buffer.
This works fine for ANSI/MBCS, but fails for Unicode.
From this point of view, I think that StringCbCopy can be "safer" than
StringCchCopy (because with the former, the use of sizeof() works for both
ANSI and Unicode).

But, after this discussion, the following question comes in my mind: "Why is
the OP using these C-style string functions, instead of using a convenient
string class like CString??"
(To my knowledge, these days the only reason to use pure C - at least in the
Windows world - is writing kernel-mode device drivers...)


Giovanni



From: RB on

"Giovanni Dicanio" wrote
> ........<cut out previous>......
> I think a common mistake is to use StringCchCopy with sizeof() instead of _countof() for the size of the destination buffer

Thanks Giovanni for all of the < cut out > elaboration, it helps me to
further get under the concept which will enable me to program more
correctly in any circumstance (or LIB used ).
My version of VS does not have the _countof( ) routine (macro ? )
but I found this on a search, which I surmise would enable the me
the same product (with less typing and code line length).
#define CountOf(array) (sizeof(array)/sizeof(array[0]))
.....RB


From: Joseph M. Newcomer on
Because when you use strsafe.h it enables a feature that diagnoses the obsolete and deadly
sprintf, strcpy and strcat as the errors they have always been!

What it means is that you should NOT be using these throwbacks to a Golden Age when NOBODY
ever wrote code that had buffer overrun errors.

I don't suppose it would inconvenience you to show us the line that generate the error?

How is it we are supposed to make sense of an error message when you don't show us the
line it occurred on?

I suspect the line has a sprintf in it. But that's just a guess. If you really wanted an
answer, you would have shown what was on line 164 (or better still, lines 160-165,
indicating which was line 164).

joe


On Tue, 27 Apr 2010 14:41:13 -0400, "RB" <NoMail(a)NoSpam> wrote:

>Hello, I am curious as to how my MS VS Pro ver 6.0 sp5 has gained data to give
>me the following suggestion in the error readout. In other words I had to download
>and install the PlatformSDK Feb 2003 version to even give my VS the ability to use
>the strsafe lib. So now that I have implemented the StringCchCopy function to replace
>strcpy, I get the following error and suggestive msg on a line I had sprintf on.
> Which is great, and I can accommodate the suggestion gladly, I am just curious how
>did my compiler know to give me this suggestion on this error ?? Instead of just telling
>me that I had an "undeclared identifier" which is what I find when I look up the error
>in my VS error lookup, the compiler now actually tells me this additional strsafe function
>suggestion, that is not found anywhere in the error lookup data ?
>----------compile results paste
>Compiling...
>Loan2View.cpp
>Z:\PROGRAMMING\VC_6_Projects\Loan2\Loan2View.cpp(164) : error C2065: 'sprintf_instead_use_StringCbPrintfA_or_StringCchPrintfA' :
>undeclared identifier
>Loan2.exe - 1 error(s), 0 warning(s)
>
Joseph M. Newcomer [MVP]
email: newcomer(a)flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
From: Giovanni Dicanio on
"RB" <NoMail(a)NoSpam> wrote:

> Thanks Giovanni for all of the < cut out > elaboration, it helps me to
> further get under the concept which will enable me to program more
> correctly in any circumstance (or LIB used ).

RB: You are welcome.

> My version of VS does not have the _countof( ) routine (macro ? )
> but I found this on a search, which I surmise would enable the me
> the same product (with less typing and code line length).
> #define CountOf(array) (sizeof(array)/sizeof(array[0]))

Probably you are using something older than VS2005 (a.k.a. VC8).

Yes, _countof() can be defined using preprocessor macro like you did above.
But if you are using C++ (not pure C) and a modern C++ compiler that
understands templates well, then you can use a template metaprogramming
trick to make _countof() fail if a pointer is passed as argument, instead of
a static array, as described here:

http://groups.google.com/group/microsoft.public.vc.mfc/msg/3f20f126165d6a3f

Giovanni