mirror
/
openwrt
mirror of https://git.openwrt.org/openwrt/openwrt.git
1
0
Fork 0
Code Releases Activity

uci: Backport security fixes 

This packports two security fixes from master.

Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
(cherry picked from commit f9005d4f80)
This commit is contained in:
Hauke Mehrtens 2020-10-28 00:16:38 +01:00
parent d94c59f7eb
commit 5625f5bc36
3 changed files with 164 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
package/system/uci/Makefile
View File

@ -9,7 +9,7 @@
include $(TOPDIR)/rules.mk
PKG_NAME:=uci
PKG_RELEASE:=1
PKG_RELEASE:=2
PKG_SOURCE_URL=$(PROJECT_GIT)/project/uci.git
PKG_SOURCE_PROTO:=git

51
package/system/uci/patches/001-file-uci_parse_package-fix-heap-use-after-free.patch Normal file
View File

@ -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
@@ -387,8 +387,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;

112
package/system/uci/patches/002-file-Check-buffer-size-after-strtok.patch Normal file
View File

@ -0,0 +1,112 @@
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
@@ -28,6 +28,7 @@
#include <glob.h>
#include <string.h>
#include <stdlib.h>
+#include <errno.h>
#include "uci.h"
#include "uci_internal.h"
@@ -63,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;
@@ -120,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
*/
@@ -384,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);
@@ -416,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);
@@ -466,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)