[openwrt/openwrt] uci: Backport security fixes
LEDE Commits
lede-commits at lists.infradead.org
Wed Oct 28 18:25:07 EDT 2020
hauke pushed a commit to openwrt/openwrt.git, branch openwrt-19.07:
https://git.openwrt.org/78c4c04dd7979a7f6d3cadeb1783b6c38d63b575
commit 78c4c04dd7979a7f6d3cadeb1783b6c38d63b575
Author: Hauke Mehrtens <hauke at hauke-m.de>
AuthorDate: Wed Oct 28 00:16:38 2020 +0100
uci: Backport security fixes
This packports two security fixes from master.
Signed-off-by: Hauke Mehrtens <hauke at hauke-m.de>
(cherry picked from commit f9005d4f80dee3dcc257d4613cbc46668faad094)
---
package/system/uci/Makefile | 2 +-
...uci_parse_package-fix-heap-use-after-free.patch | 51 ++++++++++
.../002-file-Check-buffer-size-after-strtok.patch | 104 +++++++++++++++++++++
3 files changed, 156 insertions(+), 1 deletion(-)
diff --git a/package/system/uci/Makefile b/package/system/uci/Makefile
index 1a669c3ed1..903d4c50ca 100644
--- a/package/system/uci/Makefile
+++ b/package/system/uci/Makefile
@@ -9,7 +9,7 @@
include $(TOPDIR)/rules.mk
PKG_NAME:=uci
-PKG_RELEASE:=3
+PKG_RELEASE:=4
PKG_SOURCE_URL=$(PROJECT_GIT)/project/uci.git
PKG_SOURCE_PROTO:=git
diff --git a/package/system/uci/patches/001-file-uci_parse_package-fix-heap-use-after-free.patch b/package/system/uci/patches/001-file-uci_parse_package-fix-heap-use-after-free.patch
new file mode 100644
index 0000000000..6a922278ba
--- /dev/null
+++ b/package/system/uci/patches/001-file-uci_parse_package-fix-heap-use-after-free.patch
@@ -0,0 +1,51 @@
+From a3e650911f5e6f67dcff09974df3775dfd615da6 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Petr=20=C5=A0tetiar?= <ynezz at 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 at datto.com>
+Signed-off-by: Petr Štetiar <ynezz at 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;
+
diff --git a/package/system/uci/patches/002-file-Check-buffer-size-after-strtok.patch b/package/system/uci/patches/002-file-Check-buffer-size-after-strtok.patch
new file mode 100644
index 0000000000..95cde2102b
--- /dev/null
+++ b/package/system/uci/patches/002-file-Check-buffer-size-after-strtok.patch
@@ -0,0 +1,104 @@
+From eae126f66663e5c73e5d290b8e3134449489340f Mon Sep 17 00:00:00 2001
+From: Hauke Mehrtens <hauke at 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 at hauke-m.de>
+[fixed merge conflict in tests]
+Signed-off-by: Petr Štetiar <ynezz at 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)
More information about the lede-commits
mailing list