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.kar...@fox-it.com>
---
v2: add spaces around '+'
src/openvpn/buffer.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
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)
{
--
2.7.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
