buffer_list_aggregate_separator(): prevent 0-byte malloc
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 <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>
Steffan Karger authored on 2017/12/29 18:54:00Showing 1 changed files
| ... | ... |
@@ -1251,8 +1251,7 @@ buffer_list_aggregate_separator(struct buffer_list *bl, const size_t max_len, |
| 1251 | 1251 |
struct buffer_entry *e = bl->head, *f; |
| 1252 | 1252 |
|
| 1253 | 1253 |
ALLOC_OBJ_CLEAR(f, struct buffer_entry); |
| 1254 |
- f->buf.data = malloc(size); |
|
| 1255 |
- check_malloc_return(f->buf.data); |
|
| 1254 |
+ f->buf = alloc_buf(size + 1); /* prevent 0-byte malloc */ |
|
| 1256 | 1255 |
f->buf.capacity = size; |
| 1257 | 1256 |
for (i = 0; e && i < count; ++i) |
| 1258 | 1257 |
{
|