Don't make any changes to the lhash structure if we are going to fail
authorMatt Caswell <matt@openssl.org>
Wed, 18 Oct 2017 13:07:57 +0000 (14:07 +0100)
committerMatt Caswell <matt@openssl.org>
Tue, 24 Oct 2017 09:51:56 +0000 (10:51 +0100)
commit4ce8bebcca90a1f8a3347be29df7a501043d4464
tree882dea8743ce37a52af1c0e8a579c3d83de6a90e
parent04761b557a53f026630dd5916b2b8522d94579db
Don't make any changes to the lhash structure if we are going to fail

The lhash expand() function can fail if realloc fails. The previous
implementation made changes to the structure and then attempted to do a
realloc. If the realloc failed then it attempted to undo the changes it
had just made. Unfortunately changes to lh->p were not undone correctly,
ultimately causing subsequent expand() calls to increment num_nodes to a
value higher than num_alloc_nodes, which can cause out-of-bounds reads/
writes. This is not considered a security issue because an attacker cannot
cause realloc to fail.

This commit moves the realloc call to near the beginning of the function
before any other changes are made to the lhash structure. That way if a
failure occurs we can immediately fail without having to undo anything.

Thanks to Pavel Kopyl (Samsung) for reporting this issue.

Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4550)
crypto/lhash/lhash.c