Hi,
More off Twitter:
<guidovranken> @solardiz Here is another very recent OpenSSL CVE that hasnt been released officially yet
https://github.com/openssl/openssl/commit/a004e72b95835136d3f1ea90517f706c24c03da7
| Avoid some undefined pointer arithmetic
|
| A common idiom in the codebase is:
|
| if (p + len > limit)
| {
| return; /* Too long */
| }
|
| Where "p" points to some malloc'd data of SIZE bytes and
| limit == p + SIZE
|
| "len" here could be from some externally supplied data (e.g. from a TLS
| message).
|
| The rules of C pointer arithmetic are such that "p + len" is only well
| defined where len <= SIZE. Therefore the above idiom is actually
| undefined behaviour.
|
| For example this could cause problems if some malloc implementation
| provides an address for "p" such that "p + len" actually overflows for
| values of len that are too big and therefore p + len < limit!
|
| Issue reported by Guido Vranken.
|
| CVE-2016-2177
The commit message above gives pointer wraparound as an example of when
and how this UB could manifest itself, but I think even more likely is
that an optimizing C compiler would remove the check because it can't
be reliably true (it can be either false or UB). A valid pointer is at
most one element beyond the end of an object, so in the example given
above "p + len > limit" is never reliably true. In the actual code
being patched, there are different instances of the problem, and some of
the checks look like they're effectively ">= limit" rather than "> limit".
Those ">=" checks are more lucky, as they can't be completely removed
(but can still misbehave if the pointer advances further).
In fact, there are so many instances that someone should re-review the
patch and possibly look for even more instances of the problem (maybe in
an automated way different from what might have been used so far). The
commit has "Reviewed-by: Rich Salz", which is great, but I think it
needs more eyes than the committer's and one other person's.
Alexander