* [PATCH v2 nft] scanner: fix out-of-bound memory write in include_file()
@ 2019-11-29 14:30 Eric Jallot
2019-12-02 8:28 ` Eric Jallot
2019-12-02 18:20 ` Pablo Neira Ayuso
0 siblings, 2 replies; 3+ messages in thread
From: Eric Jallot @ 2019-11-29 14:30 UTC (permalink / raw)
To: netfilter-devel; +Cc: Eric Jallot
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>
---
src/scanner.l | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/src/scanner.l b/src/scanner.l
index 80b5a5f0dafc..d32adf4897ae 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -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);
--
2.11.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 nft] scanner: fix out-of-bound memory write in include_file()
2019-11-29 14:30 [PATCH v2 nft] scanner: fix out-of-bound memory write in include_file() Eric Jallot
@ 2019-12-02 8:28 ` Eric Jallot
2019-12-02 18:20 ` Pablo Neira Ayuso
1 sibling, 0 replies; 3+ messages in thread
From: Eric Jallot @ 2019-12-02 8:28 UTC (permalink / raw)
To: netfilter-devel
I forgot to say that v2 is only an update on the commit's message.
Sorry.
Le ven. 29 nov. 2019 à 15:32, Eric Jallot <ejallot@gmail.com> a écrit :
>
> 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>
> ---
> src/scanner.l | 21 +++++++++------------
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/src/scanner.l b/src/scanner.l
> index 80b5a5f0dafc..d32adf4897ae 100644
> --- a/src/scanner.l
> +++ b/src/scanner.l
> @@ -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);
> --
> 2.11.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 nft] scanner: fix out-of-bound memory write in include_file()
2019-11-29 14:30 [PATCH v2 nft] scanner: fix out-of-bound memory write in include_file() Eric Jallot
2019-12-02 8:28 ` Eric Jallot
@ 2019-12-02 18:20 ` Pablo Neira Ayuso
1 sibling, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2019-12-02 18:20 UTC (permalink / raw)
To: Eric Jallot; +Cc: netfilter-devel
On Fri, Nov 29, 2019 at 03:30:39PM +0100, Eric Jallot wrote:
> 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)
Applied, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-02 18:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-29 14:30 [PATCH v2 nft] scanner: fix out-of-bound memory write in include_file() Eric Jallot
2019-12-02 8:28 ` Eric Jallot
2019-12-02 18:20 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).