This packports two security fixes from master. Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de> (cherry picked from commit f9005d4f80dee3dcc257d4613cbc46668faad094)master
parent
3af9c5fefd
commit
78c4c04dd7
@ -0,0 +1,51 @@ |
||||
From a3e650911f5e6f67dcff09974df3775dfd615da6 Mon Sep 17 00:00:00 2001
|
||||
From: =?UTF-8?q?Petr=20=C5=A0tetiar?= <ynezz@true.cz>
|
||||
Date: Sat, 3 Oct 2020 01:29:21 +0200
|
||||
Subject: [PATCH] file: uci_parse_package: fix heap use after free
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
Fixes following issue which is caused by usage of pointer which pointed
|
||||
to a reallocated address:
|
||||
|
||||
ERROR: AddressSanitizer: heap-use-after-free on address 0x619000000087 at pc 0x000000509aa7 bp 0x7ffd6b9c3c40 sp 0x7ffd6b9c3400
|
||||
READ of size 2 at 0x619000000087 thread T0
|
||||
#0 0x509aa6 in strdup (test-fuzz+0x509aa6)
|
||||
#1 0x7fc36d2a1636 in uci_strdup util.c:60:8
|
||||
#2 0x7fc36d29e1ac in uci_alloc_generic list.c:55:13
|
||||
#3 0x7fc36d29e241 in uci_alloc_package list.c:253:6
|
||||
#4 0x7fc36d2a0ba3 in uci_switch_config file.c:375:18
|
||||
#5 0x7fc36d2a09b8 in uci_parse_package file.c:397:2
|
||||
#6 0x7fc36d2a09b8 in uci_parse_line file.c:513:6
|
||||
#7 0x7fc36d2a09b8 in uci_import file.c:681:4
|
||||
|
||||
0x619000000087 is located 7 bytes inside of 1024-byte region [0x619000000080,0x619000000480)
|
||||
freed by thread T0 here:
|
||||
#0 0x51daa9 in realloc (test-fuzz+0x51daa9)
|
||||
#1 0x7fc36d2a1612 in uci_realloc util.c:49:8
|
||||
|
||||
previously allocated by thread T0 here:
|
||||
#0 0x51daa9 in realloc (test-fuzz+0x51daa9)
|
||||
#1 0x7fc36d2a1612 in uci_realloc util.c:49:8
|
||||
|
||||
Reported-by: Jeremy Galindo <jgalindo@datto.com>
|
||||
Signed-off-by: Petr Štetiar <ynezz@true.cz>
|
||||
---
|
||||
file.c | 2 +-
|
||||
...sig-06,src-000079,time-22005942,op-ext_AO,pos-8 | Bin 0 -> 56 bytes
|
||||
2 files changed, 1 insertion(+), 1 deletion(-)
|
||||
create mode 100644 tests/fuzz/corpus/id-000000,sig-06,src-000079,time-22005942,op-ext_AO,pos-8
|
||||
|
||||
--- a/file.c
|
||||
+++ b/file.c
|
||||
@@ -388,8 +388,8 @@ static void uci_parse_package(struct uci
|
||||
pctx->pos += strlen(pctx_cur_str(pctx)) + 1;
|
||||
|
||||
ofs_name = next_arg(ctx, true, true, true);
|
||||
- name = pctx_str(pctx, ofs_name);
|
||||
assert_eol(ctx);
|
||||
+ name = pctx_str(pctx, ofs_name);
|
||||
if (single)
|
||||
return;
|
||||
|
@ -0,0 +1,104 @@ |
||||
From eae126f66663e5c73e5d290b8e3134449489340f Mon Sep 17 00:00:00 2001
|
||||
From: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
Date: Sun, 4 Oct 2020 17:14:49 +0200
|
||||
Subject: [PATCH] file: Check buffer size after strtok()
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
This fixes a heap overflow in the parsing of the uci line.
|
||||
|
||||
The line which is parsed and put into pctx->buf is null terminated and
|
||||
stored on the heap. In the uci_parse_line() function we use strtok() to
|
||||
split this string in multiple parts after divided by a space or tab.
|
||||
strtok() replaces these characters with a NULL byte. If the next byte is
|
||||
NULL we assume that this NULL byte was added by strtok() and try to
|
||||
parse the string after this NULL byte. If this NULL byte was not added
|
||||
by strtok(), but by fgets() to mark the end of the string we would read
|
||||
over this end of the string in uninitialized memory and later over the
|
||||
allocated buffer.
|
||||
|
||||
Fix this problem by storing how long the line we read was and check if
|
||||
we would read over the end of the string here.
|
||||
|
||||
This also adds the input which detected this crash to the corpus of the
|
||||
fuzzer.
|
||||
|
||||
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
|
||||
[fixed merge conflict in tests]
|
||||
Signed-off-by: Petr Štetiar <ynezz@true.cz>
|
||||
---
|
||||
file.c | 19 ++++++++++++++++---
|
||||
tests/cram/test-san_uci_import.t | 1 +
|
||||
tests/cram/test_uci_import.t | 1 +
|
||||
.../2e18ecc3a759dedc9357b1298e9269eccc5c5a6b | 1 +
|
||||
uci_internal.h | 1 +
|
||||
5 files changed, 20 insertions(+), 3 deletions(-)
|
||||
create mode 100644 tests/fuzz/corpus/2e18ecc3a759dedc9357b1298e9269eccc5c5a6b
|
||||
|
||||
--- a/file.c
|
||||
+++ b/file.c
|
||||
@@ -64,6 +64,7 @@ __private void uci_getln(struct uci_cont
|
||||
return;
|
||||
|
||||
ofs += strlen(p);
|
||||
+ pctx->buf_filled = ofs;
|
||||
if (pctx->buf[ofs - 1] == '\n') {
|
||||
pctx->line++;
|
||||
return;
|
||||
@@ -121,6 +122,15 @@ static inline void addc(struct uci_conte
|
||||
*pos_src += 1;
|
||||
}
|
||||
|
||||
+static int uci_increase_pos(struct uci_parse_context *pctx, size_t add)
|
||||
+{
|
||||
+ if (pctx->pos + add > pctx->buf_filled)
|
||||
+ return -EINVAL;
|
||||
+
|
||||
+ pctx->pos += add;
|
||||
+ return 0;
|
||||
+}
|
||||
+
|
||||
/*
|
||||
* parse a double quoted string argument from the command line
|
||||
*/
|
||||
@@ -385,7 +395,8 @@ static void uci_parse_package(struct uci
|
||||
char *name;
|
||||
|
||||
/* command string null-terminated by strtok */
|
||||
- pctx->pos += strlen(pctx_cur_str(pctx)) + 1;
|
||||
+ if (uci_increase_pos(pctx, strlen(pctx_cur_str(pctx)) + 1))
|
||||
+ uci_parse_error(ctx, "package without name");
|
||||
|
||||
ofs_name = next_arg(ctx, true, true, true);
|
||||
assert_eol(ctx);
|
||||
@@ -417,7 +428,8 @@ static void uci_parse_config(struct uci_
|
||||
}
|
||||
|
||||
/* command string null-terminated by strtok */
|
||||
- pctx->pos += strlen(pctx_cur_str(pctx)) + 1;
|
||||
+ if (uci_increase_pos(pctx, strlen(pctx_cur_str(pctx)) + 1))
|
||||
+ uci_parse_error(ctx, "config without name");
|
||||
|
||||
ofs_type = next_arg(ctx, true, false, false);
|
||||
type = pctx_str(pctx, ofs_type);
|
||||
@@ -467,7 +479,8 @@ static void uci_parse_option(struct uci_
|
||||
uci_parse_error(ctx, "option/list command found before the first section");
|
||||
|
||||
/* command string null-terminated by strtok */
|
||||
- pctx->pos += strlen(pctx_cur_str(pctx)) + 1;
|
||||
+ if (uci_increase_pos(pctx, strlen(pctx_cur_str(pctx)) + 1))
|
||||
+ uci_parse_error(ctx, "option without name");
|
||||
|
||||
ofs_name = next_arg(ctx, true, true, false);
|
||||
ofs_value = next_arg(ctx, false, false, false);
|
||||
--- a/uci_internal.h
|
||||
+++ b/uci_internal.h
|
||||
@@ -33,6 +33,7 @@ struct uci_parse_context
|
||||
const char *name;
|
||||
char *buf;
|
||||
int bufsz;
|
||||
+ size_t buf_filled;
|
||||
int pos;
|
||||
};
|
||||
#define pctx_pos(pctx) ((pctx)->pos)
|
Loading…
Reference in new issue