file: Check buffer size after strtok()

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>

diff --git a/file.c b/file.c
index c9b23d4bbd2f98d2d08838e02a0ae7cf7ff2a2f8..93abfaeca1c385f043fdd324d5f0cab00f4bfe20 100644 (file)
--- a/file.c
+++ b/file.c
@@ -64,6 +64,7 @@ __private void uci_getln(struct uci_context *ctx, size_t offset)
                        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_context *ctx, size_t *pos_dest, size_t *pos_s
        *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_context *ctx, bool single)
        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_context *ctx)
        }
 
        /* 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_context *ctx, bool list)
                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);

diff --git a/tests/cram/test-san_uci_import.t b/tests/cram/test-san_uci_import.t
index 7faed221f93ec22c5b5374a9e6f3b35989fa71aa..6b4e6b4ad5a1e0ca60d73b68f0aa7bc97c69c8f9 100644 (file)
--- a/tests/cram/test-san_uci_import.t
+++ b/tests/cram/test-san_uci_import.t
@@ -7,6 +7,7 @@ check that uci import is producing expected results:
   $ for file in $(LC_ALL=C find $FUZZ_CORPUS -type f | sort ); do
   >   uci-san import -f $file; \
   > done
+  uci-san: Parse error (package without name) at line 0, byte 68
   uci-san: I/O error
   uci-san: Parse error (invalid command) at line 0, byte 0
   uci-san: Parse error (invalid command) at line 1, byte 18

diff --git a/tests/cram/test_uci_import.t b/tests/cram/test_uci_import.t
index 8d5ab74ed4e738890b6ba5093646bb8f58d977e6..20ebc0991f4a6a8c02ce3519aca5dafd4448acd5 100644 (file)
--- a/tests/cram/test_uci_import.t
+++ b/tests/cram/test_uci_import.t
@@ -7,6 +7,7 @@ check that uci import is producing expected results:
   $ for file in $(LC_ALL=C find $FUZZ_CORPUS -type f | sort ); do
   >   valgrind --quiet --leak-check=full uci import -f $file; \
   > done
+  uci: Parse error (package without name) at line 0, byte 68
   uci: I/O error
   uci: Parse error (invalid command) at line 0, byte 0
   uci: Parse error (invalid command) at line 1, byte 18

diff --git a/tests/fuzz/corpus/2e18ecc3a759dedc9357b1298e9269eccc5c5a6b b/tests/fuzz/corpus/2e18ecc3a759dedc9357b1298e9269eccc5c5a6b
new file mode 100644 (file)
index 0000000..e14a79f
--- /dev/null
+++ b/tests/fuzz/corpus/2e18ecc3a759dedc9357b1298e9269eccc5c5a6b
@@ -0,0 +1 @@
+                                                                    p
\ No newline at end of file

diff --git a/uci_internal.h b/uci_internal.h
index 3a94dbbcc424fcd20813dc4cfa2279617b6b29e8..34528f0495050e0652ec2c3a426eb0299daca4d4 100644 (file)
--- a/uci_internal.h
+++ b/uci_internal.h
@@ -33,6 +33,7 @@ struct uci_parse_context
        const char *name;
        char *buf;
        size_t bufsz;
+       size_t buf_filled;
        size_t pos;
 };
 #define pctx_pos(pctx)         ((pctx)->pos)