[openssl-dev] [openssl.org #3950] Standard mem* functions called with length 0 and invalid pointer arguments

[Pascal Cuoq via RT]

Recently, GCC began to assume for optimization purposes that p and q are non-null pointers when
memcpy(p, q, n); is invoked.

This means that the if is eliminated completely when compiling the following sequence of instructions:

memcpy(p, q, n);
if (!p) printf("good\n");

And this causes a problem for any programmer that would expect “good” to be printed by the
following program:

#include <string.h>

void f(void *p, void *q, size_t n) {
  memcpy(p, q, n);
  if (!p) printf("good\n");
}

int main(void) {
  f(0, 0, 0);
}

The clauses in the standard that allow GCC to “optimize” the program in this way are, in C11, 7.24.1:2 and 7.1.4.

Clause 7.24.1:2 says:

“Where an argument declared as size_t n specifies the length of the array for a function,
n can have the value zero on a call to that function. Unless explicitly stated otherwise in the description
of a particular function in this subclause, pointer arguments on such a call shall still have valid values,
as described in 7.1.4”

Clause 7.1.4 also allows compilers to assume that p and q are not pointers “one past” the end of an object:

http://stackoverflow.com/questions/25390577/is-memcpya-1-b-1-0-defined-in-c11

Since you can expect GCC developers to take as much responsibility for the vulnerabilities
introduced in previously working code when they add the optimization of assuming that p and q
are not pointers “one past” than they did when they added the optimization of assuming that p
and q are not null (i.e. none at all), it would be prudent never to call any standard function
with pointers “one past”, even when these are functions that also take a length and the length
is always 0 in these cases.

OpenSSL's bignum implementation contains two invocations of standard functions that
fail this property:

https://github.com/openssl/openssl/blob/b39fc560612984e65ec30d7f37487303bf514fb3/crypto/bn/bn_add.c#L225
https://github.com/openssl/openssl/blob/b39fc560612984e65ec30d7f37487303bf514fb3/crypto/bn/bn_mont.c#L199

These two lines are actually reached with pointers “one past” and sizes of 0 during real executions.

The prudent thing to do would be to guard these lines so that the standard function is not called
with a pointer “one past”, which can be done as simply as:

if (max - r->top) memset(&rp[r->top], 0, sizeof(*rp) * (max - r->top));

if (dif) memcpy(rp, ap, sizeof(*rp) * dif);
_______________________________________________
openssl-bugs-mod mailing list
openssl-bugs-mod at openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-bugs-mod