]> git.ipfire.org Git - thirdparty/nftables.git/commitdiff
scanner: fix out-of-bound memory write in include_file()
authorEric Jallot <ejallot@gmail.com>
Fri, 29 Nov 2019 14:30:39 +0000 (15:30 +0100)
committerPablo Neira Ayuso <pablo@netfilter.org>
Mon, 2 Dec 2019 18:25:22 +0000 (19:25 +0100)
Before patch:
 # echo 'include "/tmp/rules.nft"' > /tmp/rules.nft
 # nft -f /tmp/rules.nft
 In file included from /tmp/rules.nft:1:1-25:
                  from /tmp/rules.nft:1:1-25:
 [snip]
                  from /tmp/rules.nft:1:1-25:
 /tmp/rules.nft:1:1-25: Error: Include nested too deeply, max 16 levels
 include "/tmp/rules.nft"
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 double free or corruption (out)
 Aborted (core dumped)

valgrind reports:

==8856== Invalid write of size 8
==8856==    at 0x4E8FCAF: include_file (scanner.l:718)
==8856==    by 0x4E8FEF6: include_glob (scanner.l:793)
==8856==    by 0x4E9985D: scanner_include_file (scanner.l:875)
==8856==    by 0x4E89D7A: nft_parse (parser_bison.y:828)
==8856==    by 0x4E765E1: nft_parse_bison_filename (libnftables.c:394)
==8856==    by 0x4E765E1: nft_run_cmd_from_filename (libnftables.c:497)
==8856==    by 0x40172D: main (main.c:340)

So perform bounds checking on MAX_INCLUDE_DEPTH before writing.

After patch:
 # nft -f /tmp/rules.nft
 In file included from /tmp/rules.nft:1:1-25:
                  from /tmp/rules.nft:1:1-25:
 [snip]
                  from /tmp/rules.nft:1:1-25:
 /tmp/rules.nft:1:1-25: Error: Include nested too deeply, max 16 levels
 include "/tmp/rules.nft"
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 # echo $?
 1

Also:
Update scanner_push_file() function definition accordingly.

Fixes: 32325e3c3fab4 ("libnftables: Store top_scope in struct nft_ctx")
Signed-off-by: Eric Jallot <ejallot@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
src/scanner.l
tests/shell/testcases/include/0016maxdepth_0 [new file with mode: 0755]

index 80b5a5f0dafcfdcaa7df186481cc0ffc28ca3f7c..d32adf4897ae19670f7daeabd79e143fe28b5c6b 100644 (file)
@@ -672,17 +672,13 @@ static void scanner_pop_buffer(yyscan_t scanner)
        state->indesc = state->indescs[--state->indesc_idx];
 }
 
-static struct error_record *scanner_push_file(struct nft_ctx *nft, void *scanner,
-                                             const char *filename, const struct location *loc)
+static void scanner_push_file(struct nft_ctx *nft, void *scanner,
+                             const char *filename, const struct location *loc)
 {
        struct parser_state *state = yyget_extra(scanner);
        struct input_descriptor *indesc;
        YY_BUFFER_STATE b;
 
-       if (state->indesc_idx == MAX_INCLUDE_DEPTH)
-               return error(loc, "Include nested too deeply, max %u levels",
-                            MAX_INCLUDE_DEPTH);
-
        b = yy_create_buffer(nft->f[state->indesc_idx], YY_BUF_SIZE, scanner);
        yypush_buffer_state(b, scanner);
 
@@ -697,8 +693,6 @@ static struct error_record *scanner_push_file(struct nft_ctx *nft, void *scanner
        state->indescs[state->indesc_idx] = indesc;
        state->indesc = state->indescs[state->indesc_idx++];
        list_add_tail(&indesc->list, &state->indesc_list);
-
-       return NULL;
 }
 
 static int include_file(struct nft_ctx *nft, void *scanner,
@@ -708,6 +702,12 @@ static int include_file(struct nft_ctx *nft, void *scanner,
        struct error_record *erec;
        FILE *f;
 
+       if (state->indesc_idx == MAX_INCLUDE_DEPTH) {
+               erec = error(loc, "Include nested too deeply, max %u levels",
+                            MAX_INCLUDE_DEPTH);
+               goto err;
+       }
+
        f = fopen(filename, "r");
        if (f == NULL) {
                erec = error(loc, "Could not open file \"%s\": %s\n",
@@ -715,10 +715,7 @@ static int include_file(struct nft_ctx *nft, void *scanner,
                goto err;
        }
        nft->f[state->indesc_idx] = f;
-
-       erec = scanner_push_file(nft, scanner, filename, loc);
-       if (erec != NULL)
-               goto err;
+       scanner_push_file(nft, scanner, filename, loc);
        return 0;
 err:
        erec_queue(erec, state->msgs);
diff --git a/tests/shell/testcases/include/0016maxdepth_0 b/tests/shell/testcases/include/0016maxdepth_0
new file mode 100755 (executable)
index 0000000..89eb13c
--- /dev/null
@@ -0,0 +1,8 @@
+#!/bin/bash
+
+set -e
+
+tmpfile=$(mktemp)
+
+echo 'include "/tmp/rules.nft"' > $tmpfile
+$NFT -f $tmpfile || exit 0