Usage of Secure C (memcpy_s, strcpy_s etc) functions on OpenSSL

Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Usage of Secure C (memcpy_s, strcpy_s etc) functions on OpenSSL

Raja ashok

Hi All,

 

We are using OpenSSL in our projects and we found some of the C standard functions (like memcpy, strcpy) used in OpenSSL may induce security vulnerablities like buffer overflow. Currently we have not found any instances which causes such issues.

 

But we feel better to change these calls to C11 standard's secure functions like memcpy_s, strcpy_s etc. By defining a secure calls method (list of func pointers) and allowing application to register the method. I understand that this affects performance because of return value check added for xxxx_s calls, but this will make sure it removes buffer overflow kind of issues completely from code. And also currently using secure c calls is a general industry practice.

 

Please share your opinion on it, and if any discussion happened in OpenSSL community to do this change in future.

 

Thanks in advance.

Raja Ashok

Reply | Threaded
Open this post in threaded view
|

Re: Usage of Secure C (memcpy_s, strcpy_s etc) functions on OpenSSL

Michael Wojcik
The Appendix K functions (memcpy_s, etc) do NOT "remove buffer overflow kind of issues completely", and anyone who thinks they do is making a serious error. The Appendix K functions impose an additional check. That's all they do. It is possible, and in some use cases quite easy, for the developer to pass the wrong value for the destsz parameter and invalidate that check.

Some C experts have argued that the length-checking versions of the library functions, either the C90 ones such as strncat or the Appendix K ones, are essentially pointless anyway; that the caller needs to handle truncation and so ought to know whether truncation (or overflow) would occur before attempting the operation.

On some platforms there are issues with using the Appendix K functions, either because the major C implementations for that platform do not implement them (they predate C99, or didn't implement Appendix K which was optional in C99), or because they have limitations. For example, with at least some versions of the Solaris C runtime they can't be safely used in multithreaded applications because the Runtime Constraint Handler is not thread-safe.

Reply | Threaded
Open this post in threaded view
|

Re: Usage of Secure C (memcpy_s, strcpy_s etc) functions on OpenSSL

JordanBrown
On 11/26/2019 7:41 AM, Michael Wojcik wrote:
The Appendix K functions (memcpy_s, etc) do NOT "remove buffer overflow kind of issues completely", and anyone who thinks they do is making a serious error. The Appendix K functions impose an additional check. That's all they do. It is possible, and in some use cases quite easy, for the developer to pass the wrong value for the destsz parameter and invalidate that check.

Some C experts have argued that the length-checking versions of the library functions, either the C90 ones such as strncat or the Appendix K ones, are essentially pointless anyway; that the caller needs to handle truncation and so ought to know whether truncation (or overflow) would occur before attempting the operation.

I was initially a fan of them when I first heard of them, but have since soured on them, as have others.  They are very nearly useless for libraries, because their behavior is controlled on a process-global basis.  The library cannot assume that the "bad" cases will result in aborts, because the application might have chosen to have them return errors instead.  That means that the library has to check for and handle all of those "should be impossible" error cases.

Here's a paper on the subject:  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm
-- 
Jordan Brown, Oracle ZFS Storage Appliance, Oracle Solaris
Reply | Threaded
Open this post in threaded view
|

Re: Usage of Secure C (memcpy_s, strcpy_s etc) functions on OpenSSL

Phillip Susi
In reply to this post by Michael Wojcik

Michael Wojcik writes:

> Some C experts have argued that the length-checking versions of the library functions, either the C90 ones such as strncat or the Appendix K ones, are essentially pointless anyway; that the caller needs to handle truncation and so ought to know whether truncation (or overflow) would occur before attempting the operation.

Isn't this normally/easilly handled simply by passing sizeof( buffer ) -
1?  Then the last byte is always \0 whether or not the copy was truncated.
Reply | Threaded
Open this post in threaded view
|

Re: Usage of Secure C (memcpy_s, strcpy_s etc) functions on OpenSSL

Paul Smith
In reply to this post by JordanBrown
On Tue, 2019-11-26 at 23:47 +0000, Jordan Brown wrote:
> Here's a paper on the subject:  
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm

I love the fact that the "correct and safe" example they give in
"Unnecessary Uses" is neither correct nor safe (it has a potential DOS due
to memory leak).

However I definitely do agree that the Appendix K functions are of marginal
use _at best_... I don't use them or recommend them myself.

Reply | Threaded
Open this post in threaded view
|

Re: Usage of Secure C (memcpy_s, strcpy_s etc) functions on OpenSSL

Michael Wojcik
In reply to this post by Phillip Susi
> From: Phillip Susi <[hidden email]>
> > Michael Wojcik writes:

> > Some C experts have argued that the length-checking versions of the library functions, either the C90
> > ones such as strncat or the Appendix K ones, are essentially pointless anyway; that the caller needs to
> > handle truncation and so ought to know whether truncation (or overflow) would occur before
> > attempting the operation.

> Isn't this normally/easilly handled simply by passing sizeof( buffer ) -
> 1?  Then the last byte is always \0 whether or not the copy was truncated.

No, for a number of reasons:
- The buffer may be dynamically allocated, in which case sizeof buffer is very much the wrong thing.
- A trailing nul byte is not always desirable (not everything is a string), and not all of the Appendix K
functions will append one. Neither will strncpy, if the limit is reached; strncpy has broken semantics.
- The buffer size (less 1) is the wrong value to pass when using strncat with a destination that does not start
with a nul character. Developers also frequently miss the fact that strncat can append length+1 bytes to
the buffer. (strncat's semantics are dangerously non-obvious.)
- Non-trivial programs *will still have to handle truncation properly*. It's rarely the case for a non-trivial
program that "silently truncate the data" is a good way to handle it. A well-behaved program will treat
truncation as an error or a special case that needs further handling, or at least needs to be flagged to the
user or some other diagnostic sink. The C90 length-checked functions don't communicate truncation to
the caller; the Annex K ones only return "a non-zero value" if a constraint is violated, so there's no
standardization as to how a particular constraint violation is communicated.

Also, sizeof is an operator, not a function. There's no reason to parenthesize its argument unless the
argument is a type name, in which case it's parenthesized as a special syntactic case.

Incidentally, I misremembered; it's Annex K, not Appendix K, and it was introduced in C11, not C99. So the
Annex K functions weren't even standardized until C11, and they're still optional there (though Annex K is
normative).
Reply | Threaded
Open this post in threaded view
|

Re: Usage of Secure C (memcpy_s, strcpy_s etc) functions on OpenSSL

OpenSSL - User mailing list
In reply to this post by Phillip Susi
Unless buffer is a char* instead of a char[] in which case its completely wrong.   A very common case among buggy C code.



On Wed, Nov 27, 2019 at 7:09 AM Phillip Susi <[hidden email]> wrote:

Michael Wojcik writes:

> Some C experts have argued that the length-checking versions of the library functions, either the C90 ones such as strncat or the Appendix K ones, are essentially pointless anyway; that the caller needs to handle truncation and so ought to know whether truncation (or overflow) would occur before attempting the operation.

Isn't this normally/easilly handled simply by passing sizeof( buffer ) -
1?  Then the last byte is always \0 whether or not the copy was truncated.
Reply | Threaded
Open this post in threaded view
|

Re: Usage of Secure C (memcpy_s, strcpy_s etc) functions on OpenSSL

Phillip Susi
In reply to this post by Michael Wojcik

Michael Wojcik writes:

> No, for a number of reasons:
> - The buffer may be dynamically allocated, in which case sizeof buffer
> is very much the wrong thing.

Obviously then you would use whatever variable you have the buffer size
in rather than sizeof.  The point is that you just tell strncpy 1 less
and make sure the buffer starts off zeroed ( so use calloc ) and then
you don't have to worry about strncpy not null terminating if it was too long.

> - A trailing nul byte is not always desirable (not everything is a string), and not all of the Appendix K
> functions will append one. Neither will strncpy, if the limit is
> reached; strncpy has broken semantics.

I fail to see where there is any issue to discuss when you aren't using
null terminated strings.  If you are just using memcpy, then you know
exactly how many bytes you want to copy and that is exactly how many
gets copied.  How can you screw that up that needs a _s version to
protect you from?

> - The buffer size (less 1) is the wrong value to pass when using strncat with a destination that does not start
> with a nul character. Developers also frequently miss the fact that strncat can append length+1 bytes to
> the buffer. (strncat's semantics are dangerously non-obvious.)

Obviously for strncat you need to subtract strlen() first.

> - Non-trivial programs *will still have to handle truncation properly*. It's rarely the case for a non-trivial
> program that "silently truncate the data" is a good way to handle it. A well-behaved program will treat
> truncation as an error or a special case that needs further handling, or at least needs to be flagged to the
> user or some other diagnostic sink. The C90 length-checked functions don't communicate truncation to
> the caller; the Annex K ones only return "a non-zero value" if a constraint is violated, so there's no
> standardization as to how a particular constraint violation is communicated.

Makes sense in theory.. I've just never seen a program bother in
practice.  This may be one of those differences between acadamia and the
real world.

> Also, sizeof is an operator, not a function. There's no reason to parenthesize its argument unless the
> argument is a type name, in which case it's parenthesized as a special syntactic case.

I'm aware, but virtually all C code I've seen since I learned it in the
'90s has used the parethesis.  On the very rare occasion I see someone
not do it, it feels weird.