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>
Wed, 25 Oct 2017 10:12:05 +0000 (11:12 +0100)
commit1aa0fcfb0a2bae287c2f26fe82d5b792fcc0cf3f
tree300a7007b691451f253b2c4d6e44d7bc3f55c08c
parent65d414434aeecd5aa86a46adbfbcb59b4344503a
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: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4551)
crypto/lhash/lhash.c