More secure than strncpy()? strncpy_s()!



  • I was working on porting some code from VC6 to VC2k5 and I ran into a compiler warning:

    warning C4996: 'strncpy' was declared deprecated

    A quick Google search and I found that strncpy_s() is the new "secure" version of the function. What would make good 'ol strncpy() more secure? In strncpy_s() you must specify the length of the buffer you are copying to!

    Old and busted:

    char *strncpy( char *strDest, const char *strSource, size_t count ); <New hotness:=""> errno_t strncpy_s( char *strDest, size_t numberOfElements, const char *strSource, size_t count );

    </New>

    Wow Microsoft, would have never though that not only specifiing the size of the buffer and the number of characters to copy would be more secure than just specifying the number of characters to copy! That is so much better that it is worth makeing code portablity a pain in the ass!

    And it dosn't end with redundant size parameters, they also changed the return type so you can forget about a simple #define solution. Nope you'll need to implement a compatablility layer or just re-enable the real insecure functions like strcpy().

    Why does Microsoft hate us?



  • http://msdn2.microsoft.com/en-us/library/8ef0s5kh(VS.80).aspx

    Looks fine to me, although I am sure this will devolve into another C/C++ flamewar/bitchfest.

    They only deprecated it, you can still use it if you had to (just ignore or turn off the warnings)... but looking at the enhancements, I would consider taking the time to upgrade my code if were you.



  • I can only imagine that this change is to force programmers to re-evaluate all uses of the function and they'll hopefully make sure they don't have any buffer overflows in the code.

    True, strncpy already has a count parameter, but my guess is that it's actually been used like this:

    char buffer[256];

    char * input = get_input();

    strncpy(&buffer, input, strlen(input)); 



  • @Xaox said:

    Why does Microsoft hate us?
    <sarcasm>That's an awful harsh question.  They're really only concerned with your security.</sarcasm>



  • @Nandurius said:

    True, strncpy already has a count parameter, but my guess is that it's actually been used like this:

    char buffer[256];

    char * input = get_input();

    strncpy(&buffer, input, strlen(input)); 

    I doubt anyone who went to the trouble to use strncpy and then completed missed the point is going to properly use strncpy_s either. They'll use:

    strncpy_s(&buffer, strlen(buffer), input, strlen(input)); 

    and then complain that it's broken



  • Ok, I can see the point for them coming up with a new strncpy(), but I still don't understand why they had to come up with "improved" versions of various POSIX functions like access(), eof(), chmod(), etc.  It doesn't look like the function prototypes have changed.  Couldn't they just upgrade the definition of the existing POSIX functions? 



  • Xaox, I feel your pain.  I've just spent the past nine months converting our codebase from VC6.0 to VS2005.  :-(



  • @Nandurius said:

    I can only imagine that this change is to force programmers to re-evaluate all uses of the function and they'll hopefully make sure they don't have any buffer overflows in the code.

    True, strncpy already has a count parameter, but my guess is that it's actually been used like this:

    char buffer[256];

    char * input = get_input();

    strncpy(&buffer, input, strlen(input)); 

     

     

    I suspect that people who aren't mindful of buffer overruns aren't using strncpy(), but strcpy(). The extra parameter in strncpy_s() is redundant and stinks of things like "delete_user_record(int id, bool really_do_it)". 


  • Discourse touched me in a no-no place

    What's the value of <FONT face="courier new,courier">sizeof char</FONT> again?



  • I don't really see any difficulty with it.

    You still have the option of using the old functions and you're no worse off than you were before.  If you're in a situation where you can fully control the input to strncpy you don't even have to worry.  If you're not, at least you have the option (even though I more than half suspect that most folks who are really concerned would have rolled their own a long time ago).

    Far far worse of MS to have removed the option of linking with the single-threaded CRT. 



  • Hold on, let me count the WTFs:

    • <font face="Courier New">strncpy_s</font> doesn't behave like <font face="Courier New">strncpy</font>, bacause it doesn't pad the destination with NULs.
    • Rolling out another "secure" replacement when <font face="Courier New">StringCchCopy</font> and the like've been existing for a while.
    • Using <font face="Courier New">strncpy</font> as a <font face="Courier New">strcpy</font> replacement.
    • Not using Unicode functions in Windows environment. Seriously, [b]it's time[/b] already!

    @PJH said:

    What's the value of <font face="Courier New">sizeof char</font> again?

    I must be missing something, but that won't compile.

    @mfah said:

    Far far worse of MS to have removed the option of linking with the single-threaded CRT.

    ?



  • What is really annoying is how they changed snprintf:

    int snprintf( char *buffer, size_t count, const char *format [, argument] ... );

    int snprintf_s( char *buffer, size_t sizeOfBuffer, size_t count, const char *format [, argument] ... );

    The sizeOfBuffer parameter is basically a duplicate of the count parameter.



  • @Avenger said:

    The sizeOfBuffer parameter is basically a duplicate of the count parameter.
    Presuming the string isn't unicode.



  • @Lingerance said:

    @Avenger said:
    The sizeOfBuffer parameter is basically a duplicate of the count parameter.
    Presuming the string isn't unicode.
     

    Not to be pendantic, but according to the prototype that was posted, snprintf_s() is an ASCII-only function.  Microsoft uses a special datatype (e.g. TCHAR) for strings that can be either Unicode or ASCII, depending on build options.  And MS string functions that can handle Unicode (or Unicode/ASCII) would take "number of characters" as a parameter, not size of buffer (in bytes), if I'm not mistaken.  

    For example, the prototype for the wide char (unicode) _snwprintf_s looks like:

    http://msdn2.microsoft.com/en-us/library/f30dzcf6(VS.80).aspx 

    template <size_t size>
    int _snwprintf_s(
    wchar_t (&buffer)[size],
    size_t count,
    const wchar_t *format [,
    argument] ...
     ); // C++ only

    And the documentation reads:
     
    sizeOfBuffer

    The size of the storage location for output. Size in bytes for _snprintf_s or size in words for _snwprintf_s.

    Count

    Maximum number of characters to store, or _TRUNCATE.

      

    Of course, there is the version of snprintf that can be either unicode or ascii, depending on compile-time options.   You would use it like so:

     

    TCHAR buf[52]; // TCHAR is unicode or ascii, depending on compilation flags

    _sntprintf_s(buf, sizeof(buf)/buf[0], (sizeof(buf)/buf[0]) - 1, ...)

     

    (Actually, if you read the linked doc, there is a subtle difference between sizeOfBuffer, and count, not that it's incredibly important). 

     

    If anyone cares, here's an MSDN article which tries to justify the existence of these new functions:

    http://msdn2.microsoft.com/en-us/library/ms972816.aspx 

    Here is a counter-point, which argues the new functions ain't so secure after all.

    http://www.informit.com/guides/content.aspx?g=cplusplus&seqNum=260 



  • @CodeSimian said:

    template <size_t size>
    int _snwprintf_s(
    wchar_t (&buffer)[size],
    size_t count,
    const wchar_t *format [,
    argument] ...
     ); // C++ only

     

    Sorry, posted the wrong prototype (I am the real WTF):

    int _snwprintf_s(
    wchar_t *buffer,
    size_t sizeOfBuffer,
    size_t count,
    const wchar_t *format [,
    argument] ...
    );

     Long story short, both sizeOfBuffer and count will always refer to the same unit of measure: either bytes (sizeof(ascii char)) or "words" (sizeof(wide char)).  If you read the doc, the semantics for each are slightly different, though.



  •  Without reading any documentation, or even the rest of the messages on this thread, I'm going to guess that strncpy_s clears out the unused portion of the buffer.  That is certainly more secure.

    For example:


    char outputBuffer[25];
    char password[] = "swordfish";
    char foo[] = "foo";

    sprintf(outputBuffer, "My password is %s", password);
    //output some stuff
    strncpy(outputBuffer, foo, 4);
    //ouput some stuff

    //insert stupid code that writes all of outputBuffer to a file here for some reason

    If you look at that file, you'll know that "foo\0assword is swordfish".  But not with strncpy_s



  • @CodeSimian said:

    TCHAR buf[52]; // TCHAR is unicode or ascii, depending on compilation flags

    _sntprintf_s(buf, sizeof(buf)/buf[0], (sizeof(buf)/buf[0]) - 1, ...)

     

    Sorry, another WTF.  Obviously, that should read 

    TCHAR buf[52]; // TCHAR is unicode or ascii, depending on compilation flags

    _sntprintf_s(buf, sizeof(buf)/sizeof(buf[0]), (sizeof(buf)/sizeof(buf[0])) - 1, ...)



  •  @vt_mruhlin said:

    Without reading any documentation, or even the rest of the messages on this thread, I'm going to guess that strncpy_s clears out the unused portion of the buffer.  That is certainly more secure.

    'fraid not. (You were serious, right?)

    @MSDN said:

    For example,

    char dst[5];

    strncpy_s(dst, 5, "a long string", 5);

    means that we are asking strncpy_s to copy five characters into a buffer five bytes long; this would leave no space for the null terminator, hence strncpy_s zeroes out the string and calls the invalid parameter handler.

    If truncation behavior is needed, use _TRUNCATE or (size – 1):

    strncpy_s(dst, 5, "a long string", _TRUNCATE);

    strncpy_s(dst, 5, "a long string", 4);

    Note that unlike strncpy, if count is greater than the length of strSource, the destination string is NOT padded with null characters up to length count.

     



  • Well, looks like it doesn't do that even though it should.

    Here's a fancy fun fact about strncpy_s though:

    If no null character was copied, the last character of the destination character array is set to a null character.

    I wish strncpy did that, but again I'd rather have it insert the null after the end of the source string.   Say if i did...


    sprintf(outputBuffer, "My password is %s", password);
    outputBuffer[24]='X'; // HAHA, no more null terminator!
    strncpy_s(outputBuffer, 25, foo, 2);
    printf("%s",  outputBuffer);

    The user would see:
    "fo password is swordfish"



  •  /me goes back and reads strncpy documentation too

    I didn't know it padded up to count.  I thought it just went until it was done with the first string, then put a single \0 there.  The more you know. 



  • @vt_mruhlin said:

    Here's a fancy fun fact about strncpy_s though:

    If no null character was copied, the last character of the destination character array is set to a null character.

    I wish strncpy did that, but again I'd rather have it insert the null after the end of the source string.  

     

    Actually, the only possible way for no null character to be copied is if strlen(sourceString) >= sizeof(destBuffer), so the "extra" null cannot be inserted after the end of the source string, otherwise you'd overflow the destination buffer.  If strlen(sourceString) < sizeof(destBuffer), then naturally the terminating null would be copied from the source.

    But, yes, the regular strncpy is a WTF when strlen(sourceString) >= sizeof(destBuffer), since you end up with a non-null terminated string.  I usually write a wrapper for strncpy ("safe" strncpy) that only copies at most sizeof(destBuffer)-1 bytes and guarantees null termination.



  • Theres an old ruby saying: Types don't need to be checked at compile time (ruby is after all a strongly typed language, you just don't declare them) you should have test cases to test if your code works (and does not get a class cast exception) or does buffer overflow errors!

     

    strcpy vs strncpy vs strncpy_s... My f-ing god! thank god i program in Java to make a living. (not that java is that brilliant) but at least we don't have this problem...

    To be honest, I get the feeling that this is just to make you more MS dependent as only their compiler might interperete this code.

    I agree however with the post that claims strncpy_s is just like doing delete_user(int id, boolean really_do_it), if you don't test your code then complain that MS should fix all your coding errors in their compiler you don't deserve having the right to program.

    Plus sometimes people know that objects are not nul, why make redundent if statements in the freaken tiny functions??? Read that on the ms help...



  • @Xaox said:

    Wow Microsoft, would have never though that not only specifiing the size of the buffer and the number of characters to copy would be more secure than just specifying the number of characters to copy! That is so much better that it is worth makeing code portablity a pain in the ass!

     

    Allow me to introduce you to my good friend:

    #define _CRT_SECURE_NO_WARNINGS

    Put it at the top of your code.



  • @dlikhten said:

    To be honest, I get the feeling that this is just to make you more MS dependent as only their compiler might interperete this code.
    Bullshit. As it is a trivial function you can rewrite it for any platform that doesn't have it, just wrap it in a #ifdef/#endif. Many non-standard C functions are used in this manner when portability is important.
    @dlikhten said:
    Plus sometimes people know that objects are not nul, why make redundent if statements in the freaken tiny functions??? Read that on the ms help...

    Some functions seg-fault if you pass them a NULL. Sometimes the function has to have those redundant if NULLs because people don't check them before passing.



  • @dlikhten said:

    I agree however with the post that claims strncpy_s is just like doing delete_user(int id, boolean really_do_it), if you don't test your code then complain that MS should fix all your coding errors in their compiler you don't deserve having the right to program.
     

    Yes, but the standard strncpy() is not guaranteed to nul-terminate the string.  From the GNU strncpy() man page:

     The strncpy() function is similar, except that not more than n bytes of
     src  are copied. Thus, if there is no null byte among the first n bytes
     of src, the result will not be null-terminated.

    So, the following code is always a WTF:

     char buf[30]

    strncpy(buf, some_untrusted_source, sizeof(buf));

    printf("%s", buf); /* buf may not be null-terminated.  Your code could crash. */

     What's the alternative?

    strncpy(buf, some_untrusted_source, sizeof(buf));

    buf[sizeof(buf)-1] = '\0'; 

    Or, you could wrap those 2 statements in a "safe" strncpy() function.  Or, if your program is not portable, you could use the MS extensions.   You know, gcc has compiler extensions which break portability too.  And there are UNIX  standard function calls which are not available on non-UNIX systems.  (Why do you think cygwin was invented?)

    @dlikhten said:

    Plus sometimes people know that objects are not nul, why make redundent if statements in the freaken tiny functions??? Read that on the ms help...

    ???.  Not sure how this is relevant to the discussion.  (Sorry if I misunderstood).  The only possibly valid complaint is the redundant sizeOfBuffer (size of destination buffer) and count (number of characters to copy from source) parameters.  The standard strncpy() is broken, plain and simple.  

      



  • @dlikhten said:

    Theres an old ruby saying: Types don't need to be checked at compile time (ruby is after all a strongly typed language, you just don't declare them) you should have test cases to test if your code works (and does not get a class cast exception) or does buffer overflow errors!

    Would that possibly be because Ruby is a dynamically typed language and has no way of checking types at all?

     

    @dlikhten said:

    strcpy vs strncpy vs strncpy_s... My f-ing god! thank god i program in Java to make a living. (not that java is that brilliant) but at least we don't have this problem...

    No, just String vs. StringBuffer nonsense.  Seriously, if you're writing something in C you are probably doing so for 1) compatibility with an existing codebase or 2) performance.  Number 2 means managing your own memory.  It's a pain in the ass, but there's a reason for it.

     

    @dlikhten said:

    To be honest, I get the feeling that this is just to make you more MS dependent as only their compiler might interperete this code.

    Unlike the custom extensions that exist in GCC or practically any other C compiler.  If you are writing something that has to be ported between different OSes, you will write to the lowest common denominator or use the preprocessor to define different implementations.  Unless you think the only permissible way to add functionality to a language is through the buearacracy of a standards committee.  If you don't like it, don't use it.

     

    Still, I agree that this function doesn't seem that useful.  Any competent C programmer should be checking their buffer sizes before writing to them and should be using memory-protection techniques to avoid stack or heap overflows.  It also has the bizarre pitfall of not appending a null byte if the destination string is smaller than the copied string.  This avoids your common buffer overflow in exchange for a more exotic, interesting memory corruption methodology.  At least the STL string class appends a null when converting to a C string.. 



  • Goddammit, CodeSimian, you and Lingerance beat me to the punch.  However, I must point out one thing:

     

    @CodeSimian said:

    Yes, but the standard strncpy() is not guaranteed to nul-terminate the string.  From the GNU strncpy() man page:

    strncpy() isn't a "safe" function and doesn't pretend to be.  It just blindly copies as many bytes as you tell it to, which means the Microsoft function is kind of stupid because it supposedly exists to prevent buffer overflows but only covers half of the problem.  I also disagree that strncpy is broken.  Yes, it gives you enough rope to shoot yourself in the foot with enough left over to screw the pooch, but if you can't handle buffer checking, you have no business writing C.  strncpy does what it claims and nothing more.  Maybe we should just rename it copy_as_many_bytes_as_i_specify_or_until_you_reach_a_null_byte_in_the_source_even_if_it_corrupts_my_memory()...



  • @morbiuswilters said:

    strncpy() isn't a "safe" function and doesn't pretend to be.  It just blindly copies as many bytes as you tell it to...

    I also disagree that strncpy is broken.  Yes, it gives you enough rope to shoot yourself in the foot with enough left over to screw the pooch, but if you can't handle buffer checking, you have no business writing C.  strncpy does what it claims and nothing more.  Maybe we should just rename it copy_as_many_bytes_as_i_specify_or_until_you_reach_a_null_byte_in_the_source_even_if_it_corrupts_my_memory()...

     

    Yup, but how do you explain the fact that most implementations of snprintf() will always nul-terminate the destination.  (MS's _snprintf() is a notable exception).  snprintf() entered the scene much later than strncpy(), and is not fully standardized IIRC.

    If I want to blindly copy bytes, I may as well use memcpy().  (Yeah, I know it's not exactly the same thing).

    Anyway, strncpy() is what it is.  People who need something else can write their own wrapper, or use their vendor-specific extension.  Personally, when I use strncpy() to copy strings (say, from an untrusted source), I need the destination to be nul-terminated.  And strcpy() is obviously unsuitable for almost anything (see also: gets()).  What is the point of checking the source length every single time I copy a string, or nul-terminating the buffer manually, when I could just write a wrapper function that will do so for me.  Or, I could just use some vendor-specific library routine.

    The existence of buffer overflow exploits on multiple platforms throughout the years tells you that programmers need "safe" string-handling functions.  And higher-level languages completely abstract all of this stuff away.  (i.e. They don't allow you to shoot yourself in the foot.)



  •  Here's a good read - Raymond Chen on the history of strncpy():

    http://blogs.msdn.com/oldnewthing/archive/2005/01/07/348437.aspx

    @Raymond Chen said:

    Go back to the early days of UNIX. Personally, I only go back as far as System V. In System V, file names could be up to 14 characters long. Anything longer was truncated to 14. And the field for storing the file name was exactly 14 characters. Not 15. The null terminator was implied. This saved one byte.

     

    So allegedly, the idea was to not "waste" bytes.  You can see why nobody cares about that in this day and age.  In fact, most of the WTF-iness of C involves sacrificing code maintainability/in-built security to save memory/CPU cycles.  (See: nul-terminated strings in general.)  Disclaimer: I am a C/C++ programmer (for both Linux and Windows).

    Again, notice how the C99 standard snprintf() is guaranteed to nul-terminate.  

    Here's another article on sprintf() versus snprintf():

    http://www.gotw.ca/publications/mill19.htm 

     



  • @CodeSimian said:

    Yup, but how do you explain the fact that most implementations of snprintf() will always nul-terminate the destination.  (MS's _snprintf() is a notable exception).  snprintf() entered the scene much later than strncpy(), and is not fully standardized IIRC.

    Um, yeah, snprintf() is a newer, "safe" function.  That's why it exists.  And unlike strncpy_s(), it always terminates the destination string, so it is a lot closer to being "safe".  Still, it will truncate your data which will most likely fuck something up, so it only really saves you from memory corruption.  A much better strategy is to always handle the copies yourself and use memory-protection to segfault the program if you write out-of-bounds.  You know, like I suggested.  This way you can catch truncate bugs early and if they slip into production code, nobody can exploit them.  Still, you shouldn't be mindlessly truncating strings.  That's why you have to do the checks yourself.

     

    @CodeSimian said:

    If I want to blindly copy bytes, I may as well use memcpy().  (Yeah, I know it's not exactly the same thing).

    If you know it's not the same, why do you even bother mentioning it?

     

    @CodeSimian said:

    Anyway, strncpy() is what it is.  People who need something else can write their own wrapper, or use their vendor-specific extension.  Personally, when I use strncpy() to copy strings (say, from an untrusted source), I need the destination to be nul-terminated.  And strcpy() is obviously unsuitable for almost anything (see also: gets()).  What is the point of checking the source length every single time I copy a string, or nul-terminating the buffer manually, when I could just write a wrapper function that will do so for me.  Or, I could just use some vendor-specific library routine.

    Because the libc functions predate most modern practices?  I never said you shouldn't write a macro or wrapper function or use a vendor extension.  But at some level, you will be using strncpy().  I would hardly call it useless.  Especially if you are copying from trusted data that you already know the length of.

     

    @CodeSimian said:

    The existence of buffer overflow exploits on multiple platforms throughout the years tells you that programmers need "safe" string-handling functions.  And higher-level languages completely abstract all of this stuff away.  (i.e. They don't allow you to shoot yourself in the foot.)

    As far as I'm concerned, most programmers should be using dynamic languages with primitive strings and variable-sized arrays unless absolutely needed.  I know I sure do.  I'm not saying handling all of this low-level shit is fun, but it certainly has its place.  Sometimes I write C and sometimes I write in higher-level languages.  I try to balance performance, abstraction and development time.  I keep in mind that every line of C that I write has far more potential for damage than in a managed language.  Still, there are plenty of things I need C for.



  • @CodeSimian said:

    So allegedly, the idea was to not "waste" bytes.  You can see why nobody cares about that in this day and age.  In fact, most of the WTF-iness of C involves sacrificing code maintainability/in-built security to save memory/CPU cycles.  (See: nul-terminated strings in general.)  Disclaimer: I am a C/C++ programmer (for both Linux and Windows).

    If "nobody cares about that in this day and age" then why in God's name are you programming in C?  I would say there are plenty of people who care about it, including embedded, graphics, kernel/driver and system software developers.  When you're talking about writing the most competitive web server on the market, "wasting a few bytes" becomes a pretty big deal.

     

    Really, I don't know what you point is.  I said C strings have their place and that the C string functions are the way they are for the sake of backwards compatiblity.  There are plenty of newer methods of handling strings if you can afford the overhead or lack of portability.  I never suggested C strings were easier than any other type of string (with the exception of C++.. jesus..).  Most people should be using a managed environment with string primitives.  That's the best way to protect yourself and to keep from writing hundreds of lines of obnoxious condition checking for simple copy operations.  I'm not advocating that people just abandon their high-level languages and go back to C strings so they will end with memory corruption and "see colors, man".  But to take your 40 years of hindsight and claim that C strings are worthless is pretty silly. 



  • Note: I am not trying to flame here.  Everything here is my opinion and my opinion alone.  I have tried my best not to make personal attacks, hopefully I've succeeeded. 

    @morbiuswilters said:

    But at some level, you will be using strncpy().  I would hardly call it useless.
     

    I never said strncpy() was useless.

    @morbiuswilters said:

    If "nobody cares about that in this day and age" then why in God's name are you programming in C?  I would say there are plenty of people who care about it, including embedded, graphics, kernel/driver and system software developers.  When you're talking about writing the most competitive web server on the market, "wasting a few bytes" becomes a pretty big deal.

    Hint: I work in one (or more) of those areas. 

    So you are telling me that you have used non-nul-terminated strings to save a single byte in recent code?

    @morbiuswilters said:

    Um, yeah, snprintf() is a newer, "safe" function.  That's why it exists.

    This is where I disagree with you.  strcpy() and sprintf() are 2 different beasts, with 2 completely different purposes. One is for copying strings, the other is for .  IMO, snprintf() is not intended to be an improved version of strncpy().

     strncpy() is to strcpy() as snprintf() is to sprintf().  In other words, strncpy is the length-bounded version of strcpy.  snprintf is the length-bounded version of sprintf.  strncpy(), as we all know, does not guarante nul-termination.  Earlier, non-standardized versions of snprintf, do not guarantee nul-termination.  However, the C99 standard version of snprintf guarantees nul-termination.  Do you see the break in the pattern here?  The language implementors decided to trade the possibility of saving a single byte for each snprintf() call for the security and ease of use of an snprintf() function that always nul terminates

    @morbiuswilters said:

    And unlike strncpy_s(), it always terminates the destination string, so it is a lot closer to being "safe". 

    Are you sure about that?  I think strncpy_s() always null-terminates the destination string.  

    From MSDN:

    These functions try to copy the first D characters of strSource to strDest, where D is the lesser of count and the length of strSource. If those D characters will fit within strDest (whose size is given as numberOfElements) and still leave room for a null terminator, then those characters are copied and a terminating null is appended; otherwise, strDest[0] is set to the null character and the invalid parameter handler is invoked, as described in Parameter Validation.

     

     @morbiuswilters said:

    @CodeSimian said:

    If I want to blindly copy bytes, I may as well use memcpy().  (Yeah, I know it's not exactly the same thing).

    If you know it's not the same, why do you even bother mentioning it?

     Obviously that was a mistake.

    @morbiuswilters said:

      A much better strategy is to always handle the copies yourself and use memory-protection to segfault the program if you write out-of-bounds.  You know, like I suggested. 

    Or, you could use string handling functions that avoid segfaults altogether.  How will you handle copying strings from untrusted sources?  You will either have to restrict at input time and/or truncate the string as you copy it around.  To maintain a clean separation between UI and lower-level application code, it would be nice to have bounds-checks in both places.  (Yes, this is redundant and inefficient.)

    I am really not sure what you mean by "handle the copies" yourself.  Lets assume for the sake of argument, your app is cleanly separated from the UI and your app can make very few assumptions about user input.  (e.g. say the UI can receive strings of arbitrarily long length).  How else do you handle that except by truncation?  Are you always going to malloc a buffer that's big enough for what the user gave you?

    Really, I don't know what you point is.

    My point is sometimes people need to use C to write slightly high level code, like UIs.  (Hint: you mentioned one of those places).  I am taking about making a compromise between efficiency and security/ease of coding.  In cases like this, I think strncpy() is not really suitable. 

    I still think most people who are copying strings really don't have any use for the case where the destination string is not nul-terminated, unless like you, they are opposed to truncation.  To be honest, I don't really understand this stance.  In the case of fgets(), for example, what could you possibly do besides truncation?  (There is another function that guarantees nul-termination, IIRC). 

    Here's my other point - functions like strncpy() and especially strcpy(), are the cause of many buffer overflow bugs.  For to you blithely say "write better code" doesn't fly in the real world.  Just my opinion. 

    I guess we'll have to agree to disagree on this one. 

     

     



  • @CodeSimian said:

    strncpy() is to strcpy() as snprintf() is to sprintf().  In other words, strncpy is the length-bounded version of strcpy.  snprintf is the length-bounded version of sprintf.  strncpy(), as we all know, does not guarante nul-termination.  Earlier, non-standardized versions of snprintf, do not guarantee nul-termination.  However, the C99 standard version of snprintf guarantees nul-termination.  Do you see the break in the pattern here? 
     

    One last educated guess: if you assume that strncpy() is to strcpy() as snprintf() is to sprintf(), then it's not a stretch to guess that if the language implementors were to standardize strncpy() today, it would guarantee nul-termination.  Otherwise, why give snprintf() that treatment?  I disagree with your claim that snprintf is the evolution of strncpy.  It makes more sense to say that snprintf is the length-bounded version of sprintf, which is a distinct function from strcpy/strncpy.

    That's why I say that "nobody cares about saving a single byte in this day and age."  You might say Raymond Chen agrees with me (or I agree with him) - he repeatedly refers to strncpy's behaviour as "strange".   I am nobody, but he has accomplished a great deal, whether you like his employer or not.

    I'll just quote his blog (same link as above), and shut up (finally):

    @Raymond Chen said:

    What's the conclusion of all this? Personally, my conclusion is simply to avoid strncpy and all its friends if you are dealing with null-terminated strings. Despite the "str" in the name, these functions do not produce null-terminated strings. They convert a null-terminated string into a raw character buffer. Using them where a null-terminated string is expected as the second buffer is plain wrong. Not only do you fail to get proper null termination if the source is too long, but if the source is short you get unnecessary null padding.

     

     



  • @CodeSimian said:

    This is where I disagree with you.  strcpy() and sprintf() are 2 different beasts, with 2 completely different purposes. One is for copying strings, the other is for .  IMO, snprintf() is not intended to be an improved version of strncpy().
     

    Sorry, either I had a brain fart, or the editor ate part of my post.  I should have written: "One is for copying strings, the other is for writing formatted output to a string buffer".

    I promise, I'm done spouting my opinions on this topic. 



  • @morbiuswilters said:

    Still, you shouldn't be mindlessly truncating strings.  That's why you have to do the checks yourself.

     

    Sorry, just one more observation.  Apparently, Microsoft dislikes mindless truncation as much as you do, since strncpy_s will return an error if the copied string would be truncated, unless you explicitly request truncation yourself.  And I still claim strncpy_s terminates the destination string, unless the MSDN documentation is wrong (wouldn't be the first time).

    http://msdn2.microsoft.com/en-us/library/5dae5d43(VS.80).aspx

    These functions try to copy the first D characters of strSource to strDest, where D is the lesser of count and the length of strSource. If those D characters will fit within strDest (whose size is given as numberOfElements) and still leave room for a null terminator, then those characters are copied and a terminating null is appended; otherwise, strDest[0] is set to the null character and the invalid parameter handler is invoked, as described in Parameter Validation.

    There is an exception to the above paragraph. If count is _TRUNCATE, then as much of strSource as will fit into strDest is copied while still leaving room for the terminating null which is always appended.

     


    Okay, I'm seriously done now.

     

     



  • @morbiuswilters said:

    I'm not advocating that people just abandon their high-level languages and go back to C strings so they will end with memory corruption and "see colors, man".  But to take your 40 years of hindsight and claim that C strings are worthless is pretty silly. 
     

    I was just being facetious about the "WTF-iness" of C and null-terminated strings, okay?   Why do you think I mentioned that I am a C developer?  And nowhere did I say C strings are "worthless".  You are the only one who used the words "worthless" and "useless" (again, not a flame.)


  • Discourse touched me in a no-no place

    @Spectre said:

    @PJH said:
    What's the value of <font face="Courier New">sizeof char</font> again?
    I must be missing something, but that won't compile.

    Oops - forgot that parentheses were required if it was a type <font face="courier new,courier">sizeof</font> was being applied to. (They aren't if it's a variable.)



  • Too bad there already is an improved version of strncpy: strlcpy 



  • @Maciej said:

    The extra parameter in strncpy_s() is redundant
     

    Not for the MBCS version of the function.

     



  • @gerard_ said:

    Too bad there already is an improved version of strncpy: strlcpy 
     

    But neither strlcpy nor strncpy_s are standardized.  strlcpy seems to appear on some *nix-based OSs, while strncpy_s is only for Windows.  So anyone who wants to write portable code will still need to write their implementation/wrapper.

    From the article you linked:

    Controversy

    The strlcpy and strlcat functions are controversial.[1][2] It has been noted that they are non-standard, that there are implementation differences between the BSD and Solaris implementations,[3] and that no study has demonstrated that they lead to safer or more-secure software than using standard C functions.[citation needed] Furthermore, some, including Ulrich Drepper, argue that strlcpy and strlcat make truncation errors easier for a programmer to ignore and thus can introduce more bugs than they remove;[2] consequently, these functions have not been added to the GNU C Library. Others have expressed concern regarding the risks of truncation when using any string function involving static allocation.[4]

     

    morbiuswilters, I see from the article others share your concern about truncation.  You made a good point, and now that I thought about it, truncation cannot be ignored in many cases.  



  • @CodeSimian said:

    @gerard_ said:

    Too bad there already is an improved version of
    strncpy: strlcpy 
     

    But neither strlcpy nor strncpy_s are standardized.  strlcpy seems to appear on some *nix-based OSs, while strncpy_s is only for Windows.  So anyone who wants to write portable code will still need to write their implementation/wrapper.

    <font face="Courier New">strlcpy</font> and <font face="Courier New">strlcat</font> are BSD-licensed, no need to write your own.

    I also don't get all the truncation fuzz. If you don't want your string to be truncated, you can check for it, and <font face="Courier New">strlcpy</font> is still more convenient than <font face="Courier New">strncpy</font>.

    <mini-rant>We should be using <font face="Courier New">wcslcpy</font> anyway.></mini-rant>



  • @Avenger said:

    What is really annoying is how they changed snprintf

    As I think about it, I may have run into the snprintf_s() last week and not strncpy_s(). snprintf_s() is a much better example of this WTF, but when I decided to write this up, I couldn't remember and though it was strncpy()

    I guess that you could claim that the extra "size" parameter could be used for the size of the source to prevent a crash, but it is much harder to justify specifying the size twice in snprintf_s()

    @joe_bruin said:

    Allow me to introduce you to my good friend:

    #define _CRT_SECURE_NO_WARNINGS

    Put it at the top of your code.

    Except that would disable the warnings I would get when something like strcpy() was used. The cure may be worse than the desease.

    @morbiuswilters said:

    strncpy() isn't a "safe" function and doesn't pretend to be. It just blindly copies as many bytes as you tell it to, which means the Microsoft function is kind of stupid because it supposedly exists to prevent buffer overflows but only covers half of the problem. I also disagree that strncpy is broken. Yes, it gives you enough rope to shoot yourself in the foot with enough left over to screw the pooch, but if you can't handle buffer checking, you have no business writing C. strncpy does what it claims and nothing more. Maybe we should just rename it copy_as_many_bytes_as_i_specify_or_until_you_reach_a_null_byte_in_the_source_even_if_it_corrupts_my_memory()...

    First, exactly what kind of rope are you using? Talk about mixing metaphors. :)

    Second strncpy() is more secure than strcpy(). You just need to be smart enough to use it. You must either specify count as the smallest of the buffers or just make sure that your source is null terminated. Not terminating the destination string with NULL id obviously a problem, but can be easily fixed. In the end, if you use it correctly, it you will not crash or open up a buffer overflow vunerability.



  • @Xaox said:

    Second strncpy() is more secure than strcpy(). You just need to be smart enough
    to use it. You must either specify count as the smallest of the buffers or just
    make sure that your source is null terminated. Not terminating the destination
    string with NULL id obviously a problem, but can be easily fixed. In the end, if
    you use it correctly, it you will not crash or open up a buffer overflow
    vunerability.

    You forget about NUL-padding, which is slow. <font face="Courier New">strncpy</font> wasn't supposed to be used for NUL-terminated string copying. <font face="Courier New">StringCchCopy</font>, <font face="Courier New">strcpy_s</font>/<font face="Courier New">wcscpy_s</font>/<font face="Courier New">mbscpy_s</font>, and <font face="Courier New">strlcpy</font>/hypotetical <font face="Courier New">wcslcpy</font> were.



  • @CodeSimian said:

    Here is a counter-point, which argues the new functions ain't so secure after all.
     

    char * p =new char[9];
    char fs[ ]="123456789";
    p--; //see how easy it is?
    strncpy_s(p, 10, fs, 10);//undefined behavior
    

    Oh yes that's an easy mistake to make.



  • Good God, CodeSimian, I must have struck pretty close to the mark to get you all riled up like that :-)  Your posts were very well-reasoned and I certainly don't think you are flaming, but I believe you misunderstand many of my points.

     

    @CodeSimian said:

    I never said strncpy() was useless.

    Yeah, but you sort of implied nobody should be using it.   I guess I didn't quite get that you were being facetious.

     

    Also, I never mean to imply that snprintf() and strncpy() were anything alike.  My point was that snprintf() is a "safer" version of sprintf().  strncpy() didn't end up the same, but C standards progress at a glacial rate and strlcpy() seems like a better candidate for a safe copy anyway.  That's one of the strenghts of the language, though, because it stays true to its roots of "portable assembly", but it also means C strings are full of lots of bizarre little gotchas like that.  I don't write desktop software, so my experience is only in the server and web domain.  For most tasks I am but better off using a managed environment with string, vector and hashtable primitives.  Sometimes I need the speed of C, though.  In those cases, I do use custom functions or macros to handle strings to prevent buffer overflows, missing null terminators and unexpected truncation (which is absolutely unacceptable in my case).  Additionally, using protected "canary" blocks in memory prevents any buffer overflows from being exploitable, should one be missed.

     

    @CodeSimian said:

    Are you sure about that?  I think strncpy_s() always null-terminates the destination string.

    I don't work in an MS environment and I didn't read the MSDN page.  I was only going off what some other commenter said, but I may have misinterpreted their meaning.  stncpy_s() absolutely prevents you from ending up with a non-null-terminated string.  In that case, it is an improvement over strncpy() and not a WTF at all.  I commend MS for adding it into their stdlib.

     

    @CodeSimian said:

    Here's my other point - functions like strncpy() and especially strcpy(), are the cause of many buffer overflow bugs.  For to you blithely say "write better code" doesn't fly in the real world.  Just my opinion.

    "Write better code" isn't the only option.  You can use your own string functions and add memory protection or -- best of all -- use a managed language.  But there are always tradeoffs involved.   If you cannot accept truncation of data (which in your later posts you seem to realize) then you probably cannot use strlcpy().  strncpy_s() errors in the case of truncation which is better, but if you have to handle that condition anyway, why not just check the freakin' buffer size to begin with?  It sounds like you are writing totally different software than I am, but in my case, just handling the copy myself is usually the best method.  To each his own, though..



  • @morbiuswilters said:

    Would that possibly be because Ruby is a dynamically typed language and has no way of checking types at all?

    Check the ruby site "for java developers" they say it themselves. A dynamicly typed language is something like ActionScript. You can define properties of an object at run time. In ruby you cant do Object.valueX=1 unless valueX= is a function defined in that class. Strongly typed does not mean that type is declared at compile time. In ruby you don't need to define what type a variable is, but you absolutely know the type of the object at run time.

    String vs String buffer nonsense

    it makes perfect sense. A string is an immutable object. When you get it you are GARANTEED that it can never EVER be changed so you can use it in a multi-threaded environment without ever even having a thought of "did i pass this string somewhere else?"

    A string buffer (also string builder now) is a mutable object which is used to modify strings with minimal overhead. StringBuffer is thread-safe caz everything is synchronized, StringBuilder is not, but often used in single threads so there is no synchronization overhead. Yes StringBuffer and StringBuilder are not well named. Should be "StringBuilder" and "StringBuilderTS" (ts for thread-safe), and even with that StringBuilderTS should not exist, you should synchronized on the StringBuilder object which is what most people do anyways. Java had same issue with Vectors, now ArrayLists, Vectors are array lists that have every method synchronized, now you do it once and externally and it works faster. But at some point certain code is deprecated and eventually phased out. But STRCPY vs STRNCPY vs STRNCPY_S -- this is rediculous and is on a per-function level

    bla bla bla c-mumbo jumbo

    I was wrong in my point. I completely forget that in C you need to handle OS-specifics in the pre-compiler. Sorry, java and ruby spoiled me! I never worked on production c code so I can't comment on everything C :(

     Seriously, if you're writing something in C you are probably doing so for 1) compatibility with an existing codebase or 2) performance.  Number 2 means managing your own memory.  It's a pain in the ass, but there's a reason for it.

    I suppose it kind-of makes sense because people want to hack up quick code for (1)



  • @Xaox said:

    Second strncpy() is more secure than strcpy(). You just need to be smart enough to use it. You must either specify count as the smallest of the buffers or just make sure that your source is null terminated. Not terminating the destination string with NULL id obviously a problem, but can be easily fixed. In the end, if you use it correctly, it you will not crash or open up a buffer overflow vunerability.

    Already been pointed out, but strncpy() is in no way safer than strcpy().  If you aren't doing proper bounds-checking anyway, strncpy() probably won't help you.  The function is completely oblivious to a size param that exceeds your destination buffer.  Also, you get the problem with missing null-termination if the source string was longer.  Spectre is incorrect, though, it is meant for copying null-terminated strings, that's why it halts on a null in the source string.  If your source isn't null-terminated and your size is larger than the source, you get read errors, so it's obviously meant for copying null-terminated strings.  The problem is that it doesn't have any way of knowing if it is overflowing the destination string, and the fact that it leaves off the null-termination if the source is too long.  I don't know the history here, but I believe the entire reason for the existence of strncpy() is to fill in nulls in the destination if the source is shorter, which is probably where the 'n' comes from.

     

    strncpy_s() is a safer function if used correctly because you should be specifying the length of the source and destination, not just the size of one twice.  It also refuses to truncate data and always ensures null-termination of the destination.   Still, it seems like a meaningless gesture.  If you adept at dealing with C strings you've probably already moved past this and if not, you will probably use strncpy_s() incorrectly and shoot yourself in the foot.



  • @dlikhten said:

    Check the ruby site "for java developers" they say it themselves. A dynamicly typed language is something like ActionScript. You can define properties of an object at run time. In ruby you cant do Object.valueX=1 unless valueX= is a function defined in that class. Strongly typed does not mean that type is declared at compile time. In ruby you don't need to define what type a variable is, but you absolutely know the type of the object at run time.

    Wrong.  A dynamically typed language defines types at run-time versus a statically typed language which defines them at compile-time.  I don't even know where the hell you got this definition from.  Ruby is also "duck typed" which means that as long as an operation could be permitted the type conversion happens implicitly.

     

    @dlikhten said:

    it makes perfect sense. A string is an immutable object. When you get it you are GARANTEED that it can never EVER be changed so you can use it in a multi-threaded environment without ever even having a thought of "did i pass this string somewhere else?"

    A string buffer (also string builder now) is a mutable object which is used to modify strings with minimal overhead. StringBuffer is thread-safe caz everything is synchronized, StringBuilder is not, but often used in single threads so there is no synchronization overhead. Yes StringBuffer and StringBuilder are not well named. Should be "StringBuilder" and "StringBuilderTS" (ts for thread-safe), and even with that StringBuilderTS should not exist, you should synchronized on the StringBuilder object which is what most people do anyways. Java had same issue with Vectors, now ArrayLists, Vectors are array lists that have every method synchronized, now you do it once and externally and it works faster. But at some point certain code is deprecated and eventually phased out. But STRCPY vs STRNCPY vs STRNCPY_S -- this is rediculous and is on a per-function level

    Yes.  This makes perfect sense.  The non-synchronized, mutable String object is actually a StringBuilder.  Because I am almost never writing single-threaded programs and rarely ever deal with this "concatention" gobbledy-gook, I would hate for the String type to actually be useful.  Also, I am wary of the implicit syncronization of String and StringBuffer because it seems to encourage sloppy concurrency handling.  Shouldn't the syncronization be handled by whatever is manipulating the string?  That way it becomes explicity.  I guess it's a continuation of String being a pseudo-primitive.  "It's a class, but the + operator acts differently!  And it's thread-safe, just like all the other primitives!  (Offer not valid for 64-bit types.)"

     

    @dlikhten said:

    I was wrong in my point. I completely forget that in C you need to handle OS-specifics in the pre-compiler. Sorry, java and ruby spoiled me! I never worked on production c code so I can't comment on everything C :(

     

    "I completely forgot that C is a low-level language and Ruby is a very high-level language and that the intersection between their domains is non-existent!" 

     

    If you like Ruby and it works for you, cool.  Stop pretending that it's even in competition with C, though.  And Java certainly has it's uses, but I think there are some fundamentally retarded things in the language specification.  It amuses me to see someone complaining about C strings and then advocating Java's string handling.  Hell, I think perl is faster at string manipulation than Java is, and it's a hell of a lot easier.



  •  HAHA I knew that tag would come in handy!



  •  

    ...stuff about string vs string buffer...

    Java 1.5 and 1.6 actually deals with this. They realized that people don't always use string buffer, so they have a smart way of internally caching it so the overhead of doing "string1" + "string2" is not as grand. Though I do like Ruby's way of doing things which is just concatinate.

    Ruby's duck typing is interesting because as you said, if it looks like a duck its a duck, well in Flex you can make it look like a duck, even if it did not before by making that class dynamic, javascript is an even better example. Ruby is more of a hybrid between the two.

    In any case I strayed this post off topic with this...


Log in to reply