]> git.ipfire.org Git - thirdparty/openvpn.git/commit
Fix overflow check in openvpn_decrypt()
authorSteffan Karger <steffan.karger@fox-it.com>
Wed, 29 Jul 2015 10:30:26 +0000 (12:30 +0200)
committerGert Doering <gert@greenie.muc.de>
Sat, 1 Aug 2015 14:13:55 +0000 (16:13 +0200)
commit338f11821fa7ce95c768b0cc074f3af7346b19fa
tree850f79aaa675a26bdb78fc1bbf70b636f815e0e4
parent57576475f5d6e5dd9cdb64fa0edcf283ce4a05c9
Fix overflow check in openvpn_decrypt()

Sebastian Krahmer from the SuSE security team reported that the buffer
overflow check in openvpn_decrypt() was too strict according to the
cipher update function contract:

"The amount of data written depends on the block alignment of the
encrypted data: as a result the amount of data written may be anything
from zero bytes to (inl + cipher_block_size - 1) so outl should contain
sufficient room."

This stems from the way CBC mode works, which caches input and 'flushes'
it block-wise to the output buffer.  We do allocate enough space for this
extra block in the output buffer for CBC mode, but not for CFB/OFB modes.

This patch:
 * updates the overflow check to also verify that the extra block required
   according to the function contract is available.
 * uses buf_inc_len() to double-check for overflows during en/decryption.
 * also reserves the extra block for non-CBC cipher modes.

In practice, I could not find a way in which this would fail. The plaintext
is never longer than the ciphertext, and the implementations of CBC/OFB/CBC
for AES and BF in both OpenSSL and PolarSSL/mbed TLS do not use the buffer
beyond the plaintext length when decrypting.  However, some funky OpenSSL
engine I did not check *might* use the buffer space required by the
function contract.  So we should still make sure we have enough room
anyway.

v2 - always ASSERT() on buf_inc_len().  It is a double-check so should
     really not fail, but if it fails there has been a buffer overflow.
     At that point the best thing we can do is assert out. (The primary
     check *is* handled gracefully, and just drops the packet.)

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1438165826-32762-1-git-send-email-steffan.karger@fox-it.com>
URL: http://article.gmane.org/gmane.network.openvpn.devel/9974
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit cc377dec820f9e6e7e72981013eb3857aa6ea5ce)
src/openvpn/crypto.c
src/openvpn/crypto_backend.h