| Message ID | 1514541240-19536-1-git-send-email-steffan.karger@fox-it.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | None | expand |
Hi, On 29/12/17 17:54, Steffan Karger wrote: > As pointed out in finding OVPN-05 of the cryptograpy engineering audit > (funded by Private Internet Access), buffer_list_aggregate_separator() > could perform a 0-byte malloc when called with a list of 0-length buffers > and a "" separator. If other could would later try to access that buffer > memory, this would result in undefined behaviour. To prevent this, always > malloc() 1 byte. > > To simplify as we go, use alloc_buf() to allocate the buffer. This has > the additional benefit that the actual buffer data (not the contents) is > zero-terminated, because alloc_buf() calls calloc() and we have 1 extra > byte of data. > > Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> Acked-by: Antonio Quartulli <a@unstable.cc>
Your patch has been applied to the master and release/2.4 branch.
commit 748902f46260fe11cb25726d2bf93bb06ad338f2 (master)
commit ef4b827d68d8fef75b04f23e6322aa9d2d4ad8a8 (release/2.4)
Author: Steffan Karger
Date: Fri Dec 29 10:54:00 2017 +0100
buffer_list_aggregate_separator(): prevent 0-byte malloc
Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1514541240-19536-1-git-send-email-steffan.karger@fox-it.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16106.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 2656702..cfe6f2c 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -1251,8 +1251,7 @@ buffer_list_aggregate_separator(struct buffer_list *bl, const size_t max_len, struct buffer_entry *e = bl->head, *f; ALLOC_OBJ_CLEAR(f, struct buffer_entry); - f->buf.data = malloc(size); - check_malloc_return(f->buf.data); + f->buf = alloc_buf(size + 1); /* prevent 0-byte malloc */ f->buf.capacity = size; for (i = 0; e && i < count; ++i) {
As pointed out in finding OVPN-05 of the cryptograpy engineering audit (funded by Private Internet Access), buffer_list_aggregate_separator() could perform a 0-byte malloc when called with a list of 0-length buffers and a "" separator. If other could would later try to access that buffer memory, this would result in undefined behaviour. To prevent this, always malloc() 1 byte. To simplify as we go, use alloc_buf() to allocate the buffer. This has the additional benefit that the actual buffer data (not the contents) is zero-terminated, because alloc_buf() calls calloc() and we have 1 extra byte of data. Signed-off-by: Steffan Karger <steffan.karger@fox-it.com> --- v2: add spaces around '+' src/openvpn/buffer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)