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 11:18:16 +0000 (12:18 +0100)
commitb272c48f5669da6b01bdf079bc24e9ef30ea09b3
tree31cd806f9ff1af1b8abceed838bea35c7cc45d92
parent4ed22d63a7b150b2066f4fa676950786519c602b
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)

(cherry picked from commit 4ce8bebcca90a1f8a3347be29df7a501043d4464)
crypto/lhash/lhash.c