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 15:08:03 +0200 |
commit | cc377dec820f9e6e7e72981013eb3857aa6ea5ce (patch) | |
tree | 6e1ece2a42e606eccfcf20ed0d88a4d9f26852b8 /.gitattributes | |
parent | 710c439817522ac8f4dfa7411baa787c5e2e2f89 (diff) | |
download | openvpn-cc377dec820f9e6e7e72981013eb3857aa6ea5ce.tar.gz openvpn-cc377dec820f9e6e7e72981013eb3857aa6ea5ce.tar.xz openvpn-cc377dec820f9e6e7e72981013eb3857aa6ea5ce.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>
Diffstat (limited to '.gitattributes')
0 files changed, 0 insertions, 0 deletions