diff options
author | Steffan Karger <steffan.karger@fox-it.com> | 2015-07-29 12:30:26 +0200 |
---|---|---|
committer | Gert Doering <gert@greenie.muc.de> | 2015-08-01 16:13:55 +0200 |
commit | 338f11821fa7ce95c768b0cc074f3af7346b19fa (patch) | |
tree | 850f79aaa675a26bdb78fc1bbf70b636f815e0e4 /src/openvpn/otime.c | |
parent | 57576475f5d6e5dd9cdb64fa0edcf283ce4a05c9 (diff) | |
download | openvpn-338f11821fa7ce95c768b0cc074f3af7346b19fa.tar.gz openvpn-338f11821fa7ce95c768b0cc074f3af7346b19fa.tar.xz openvpn-338f11821fa7ce95c768b0cc074f3af7346b19fa.zip |
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)
Diffstat (limited to 'src/openvpn/otime.c')
0 files changed, 0 insertions, 0 deletions