strcpy_s and other *_s new safe functions

[goo...@bkelly.ws]

Visual Studio 2008, C++ recommended that I replace strcpy with strcpy_s because it is safer.  Fine, even cool.  An application had a string too long and hit the ASSERT and effectively crashed.  A rather inglorious end.  That is a bad result.  The code should not crash but should simply not copy too much.  An error can be returned, but PLEASE DO NOT CRASH THE APP.  That is really ugly.

[Nathan Ernst]

A couple of things:

1. strcpy is a C API, not C++

2. strcpy_s is an MS extension (the "_s" is there to indicate "safe"), not ISO C++

3. The assert is a slap upside the head to the developer, letting you know that you f***ed up. Fix it!.

4. Read the API docs. There are means to determine the necessary size of a buffer to copy a string.

5. What is your question? Or are you just griping that you encountered an assert due to your own fault?

On Thu, Jan 17, 2013 at 8:07 PM, <goo...@bkelly.ws> wrote:

Visual Studio 2008, C++ recommended that I replace strcpy with strcpy_s because it is safer.  Fine, even cool.  An application had a string too long and hit the ASSERT and effectively crashed.  A rather inglorious end.  That is a bad result.  The code should not crash but should simply not copy too much.  An error can be returned, but PLEASE DO NOT CRASH THE APP.  That is really ugly.

--
 
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To post to this group, send email to std-dis...@isocpp.org.
To unsubscribe from this group, send email to std-discussio...@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/?hl=en.
 
 

[goo...@bkelly.ws]

Re 1. strcpy is a C API, not C++

OK, I was directed here by someone in the know.

RE  2. strcpy_s is an MS extension (the "_s" is there to indicate "safe"), not ISO C++

OK, I was not aware of that.

RE: 2. strcpy_s is an MS extension (the "_s" is there to indicate "safe"), not ISO C++

But it is not safe, the application crashed.  That is not to say being safe means the app must not crash.  But this is a simple and important item.

Re  3. The assert is a slap upside the head to the developer, letting you know that you f***ed up. Fix it!.

Yes, I did.  However, sometimes a user or another app/method/function can do the same.  When an error can be handled safely rather than crashing the app, that is MUCH better than the crash.  In this, make that, these cases, a safe choice is to copy only the amount that is safe.  When the coder has less code to write then it is easier to write better code.

Re 5:  See above.

I understand your point of view.  But here is a place where code can be made safer with little, very little effort.  No question, just a request.

[Nicol Bolas]

On Thursday, January 17, 2013 6:32:35 PM UTC-8, goo...@bkelly.ws wrote:

Re 1. strcpy is a C API, not C++

OK, I was directed here by someone in the know.

RE  2. strcpy_s is an MS extension (the "_s" is there to indicate "safe"), not ISO C++

OK, I was not aware of that.

RE: 2. strcpy_s is an MS extension (the "_s" is there to indicate "safe"), not ISO C++

But it is not safe, the application crashed.

Actually, the application crashing is being safe. The unsafe thing to do would be to overwrite random memory, thus allowing for the possibility of a buffer overrun attack. It is preferable for an application to crash than to become a security vulnerability.

  That is not to say being safe means the app must not crash.  But this is a simple and important item.

Re  3. The assert is a slap upside the head to the developer, letting you know that you f***ed up. Fix it!.

Yes, I did.  However, sometimes a user or another app/method/function can do the same.  When an error can be handled safely rather than crashing the app, that is MUCH better than the crash.

Your suggestion of simply not copying as much is not handling the error; it's ignoring the error. You can't tell that the error even happened. You are supposed to pass correct parameters to strcpy_s; if you forgot to do so, then you need to correct that problem. Continuing on as though it isn't a problem is not what the function is for, and continuing on as though the copy had been fully performed is nothing less than lying to the programmer.

Remember: Microsoft created these functions (and the warning to stop using `strcpy` specifically because of buffer overruns and the like. By making an application fail when it exceeds the conditions of the string copy, it tells the programmer that there is a problem in the code that they need to correct. Your suggestion would do nothing to help fix code problems; and encountering this error is a code problem.

If they made it throw an exception (the traditional C++ mechanism for handling errors), you'd get the same effect since you probably weren't catching it.

If you need a string copy function that will silently copy only part of the string even if you give it improper parameters, then you can write one. But Microsoft's `_s` functions are not going to fail silently ever; that's not what they're there for.

[Alberto Barbati]

Il 18/01/2013 06:10, Nicol Bolas ha scritto:
> If you need a string copy function that will silently copy only part
> of the string even if you give it improper parameters, then you can
> write one.

...or simply use the ISO C strncpy function.

Ganesh

[Daniel Krügler]

2013/1/18 Nathan Ernst <nathan...@gmail.com>:
> A couple of things:
[..]

> 2. strcpy_s is an MS extension (the "_s" is there to indicate "safe"), not
> ISO C++

It is not ISO C++, but C11 has added a new control define

__STDC_WANT_LIB_EXT1__

that can be used to allow or disallow the availability if a rather
huge number secure library functions (including strcpy_s) that are
specified in the C11 standard.

Example:

#define __STDC_WANT_LIB_EXT1__ 1
#include <string.h>
// This prototype is available now:
errno_t strcpy_s(char * restrict s1, rsize_t s1max, const char * restrict s2);

I think this issue becomes interesting in the near future, when
post-C++11 considers to refer to the C11 specification.

- Daniel

[goo...@bkelly.ws]

Nicol Bolas wrote: Your suggestion of simply not copying as much is not handling the error; it's ignoring the error. You can't tell that the error even happened.

That is not correct.  Errors can be returned by the function and handled in the same manner as many other existing functions.  He disregards the concept that this class of errors can arise from sources other than the immediate method executing.  By having the strcpy function, (and its brethren, and within the optimized and thoroughly tested library function) limit the copying quantity to the available space, the coder can simplify his code.  That makes for fewer chances to commit errors and for better code overall.

[goo...@bkelly.ws]

The description I found for strncpy() follows:

Copies the first num characters of source to destination. If the end of the source C string (which is signaled by a null-character) is found before num characters have been copied, destination is padded with zeros until a total of num characters have been written to it.

No null-character is implicitly appended at the end of destination if source is longer than num (thus, in this case, destination may not be a null terminated C string).
<end>

I found two problems.  From the first section: filling the string with unneeded nulls consumes unnecessary time.  Why would that be an advantage?

From the second section when the caller asks to copy more chars than there is space the string is not null terminated.  I see that as a defect.  It can lead to problems later on.  How can that be an advantage?

[Lawrence Crowl]

On 1/18/13, goo...@bkelly.ws <goo...@bkelly.ws> wrote:
> The description I found for strncpy() follows:
>

> Copies the first *num* characters of *source* to *destination*.
> If the end of the *source* C string (which is signaled by
> a null-character) is found before *num* characters have been
> copied, *destination* is padded with zeros until a total of *num*

> characters have been written to it.
>
> No null-character is implicitly appended at the end of

> *destination* if *source* is longer than *num* (thus, in this
> case, *destination* may not be a null terminated C string).

>
> I found two problems. From the first section: filling the string
> with unneeded nulls consumes unnecessary time. Why would that
> be an advantage?

Because when you are copying to a fixed width field that is to be
written to disk, failing to copy the nulls could leak sensitive
information onto disk.

> From the second section when the caller asks to copy more chars
> than there is space the string is not null terminated. I see
> that as a defect. It can lead to problems later on. How can
> that be an advantage?

Because the string only needs a null if it is shorter than the field.

I do not have any direct evidence, but I believe that strncpy was
designed for copying *into* fixed-width fields. It mostly works
fine in that environment. The only exception is copying from a
shorter fixed-width field into a longer one, where you need to add
the extra nulls with memset.

--
Lawrence Crowl

[Nathan Ernst]

On Tue, Jan 22, 2013 at 8:54 PM, Lawrence Crowl <cr...@googlers.com> wrote:

I do not have any direct evidence, but I believe that strncpy was
designed for copying *into* fixed-width fields.

I can't disagree. I had thought that the functions returned the required size on failure, but I was wrong (I seem to remember a number of windows APIs that do this when trying to "return" a string to a caller's buffer that is not of sufficient size - none of the strcpy variants I've looked at do this). And, arguably, what is really the difference, in terms of correctness, in regards to copying into a fixed-width buffer versus a dynamically allocated buffer?  In regards to the dynamically allocated buffer, you could at least attempt retries in an arbitrarily increasing dynamically allocated buffer until you either get a bad alloc, or can actually hold the result - or the initial call could explicitly tell you what is necessary, so you have at most one retry/realloc.

For what it's worth, strncopy (http://linux.die.net/man/3/strncpy) doesn't seem to tell you at all whether or not source was "completely" copied - I suppose one could argue whether or not "I copied what I wanted" vs. "I wanted to copy the whole string, but this is all the space I had" in regards to error handling.  Conversely, the MS 'strcpy_s' variants return ERANGE if the destination is not sufficient to hold the result. Personally, in this regard, I prefer the MS approach. I've asked the library to copy the string, but it cannot, so tell me. Unfortunately, this API does not tell me the required amount of memory for the copy to succeed, but at least it tells me that it has not been able to completely perform the copy.

[goo...@bkelly.ws]

On Tuesday, January 22, 2013 8:54:49 PM UTC-6, Lawrence Crowl wrote:

On 1/18/13, goo...@bkelly.ws <goo...@bkelly.ws> wrote:
> The description I found for strncpy() follows:
>

<snip?

Because when you are copying to a fixed width field that is to be
written to disk, failing to copy the nulls could leak sensitive
information onto disk.

I do not agree with that.  If your application is handling sensitive information then you should null out all the strings after use.  Do not rely on a side effect of a function to do that. (the purpose of strcpy (et al) is to copy bytes, not clear out memory, so anything like that is a side effect.)

[Lawrence Crowl]

On 1/24/13, goo...@bkelly.ws <goo...@bkelly.ws> wrote:

> On January 22, 2013, Lawrence Crowl wrote:
> > Because when you are copying to a fixed width field that is
> > to be written to disk, failing to copy the nulls could leak
> > sensitive information onto disk.
>
> I do not agree with that. If your application is handling
> sensitive information then you should null out all the strings
> after use. Do not rely on a side effect of a function to do
> that. (the purpose of strcpy (et al) is to copy bytes, not clear
> out memory, so anything like that is a side effect.)

That side effect is part of the defined behavior of strncpy. If one
cannot rely on the defined behavior, there are serious problems in
the system. :-)

--
Lawrence Crowl