| Message ID | 1539209752-9882-1-git-send-email-pkusunyifeng@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3] expr: Set a limit on the depth of nested parentheses | expand |
On Wed, Oct 10, 2018 at 03:15:52PM -0700, Yifeng Sun wrote: > This patch checks the depth of nested parentheses to prevent > stack overflow. Since is_chassis_resident doesn't allow > nested parentheses, its following parentheses are not taken > into acount in the parentheses-depth context. > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10714 > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > Suggested-by: Ben Pfaff <blp@ovn.org> > --- > v1->v2: Handle parse_chassis_resident and add new test, thanks Ben! > v2->v3: Ignore parentheses from chassis resident. Thanks a lot for the updated patch. The grammar of the error message wasn't quite right, so I adjusted it. I wanted to always keep the depth counter correct, even if there was an error, so I moved the updates, like this: if (ctx->paren_depth >= MAX_PAREN_DEPTH) { lexer_error(ctx->lexer, "Parentheses nested too deeply."); return NULL; } ctx->paren_depth++; struct expr *e = expr_parse__(ctx); ctx->paren_depth--; The unbalanced parentheses in the test were confusing my editor, so I balanced them. And then I applied this to master and backported it as far as branch-2.7. Thanks again!
Thanks! On Thu, Oct 11, 2018 at 1:02 PM Ben Pfaff <blp@ovn.org> wrote: > On Wed, Oct 10, 2018 at 03:15:52PM -0700, Yifeng Sun wrote: > > This patch checks the depth of nested parentheses to prevent > > stack overflow. Since is_chassis_resident doesn't allow > > nested parentheses, its following parentheses are not taken > > into acount in the parentheses-depth context. > > > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10714 > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > > Suggested-by: Ben Pfaff <blp@ovn.org> > > --- > > v1->v2: Handle parse_chassis_resident and add new test, thanks Ben! > > v2->v3: Ignore parentheses from chassis resident. > > Thanks a lot for the updated patch. > > The grammar of the error message wasn't quite right, so I adjusted it. > > I wanted to always keep the depth counter correct, even if there was an > error, so I moved the updates, like this: > > if (ctx->paren_depth >= MAX_PAREN_DEPTH) { > lexer_error(ctx->lexer, "Parentheses nested too deeply."); > return NULL; > } > > ctx->paren_depth++; > struct expr *e = expr_parse__(ctx); > ctx->paren_depth--; > > The unbalanced parentheses in the test were confusing my editor, so I > balanced them. > > And then I applied this to master and backported it as far as > branch-2.7. Thanks again! >
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c index 148ac869e861..5880fd2e7289 100644 --- a/ovn/lib/expr.c +++ b/ovn/lib/expr.c @@ -459,6 +459,8 @@ expr_print(const struct expr *e) /* Parsing. */ +#define MAX_PAREN_DEPTH 100 + /* Context maintained during expr_parse(). */ struct expr_context { struct lexer *lexer; /* Lexer for pulling more tokens. */ @@ -466,6 +468,7 @@ struct expr_context { const struct shash *addr_sets; /* Address set table. */ const struct shash *port_groups; /* Port group table. */ bool not; /* True inside odd number of NOT operators. */ + unsigned int paren_depth; /* Depth of nested parentheses. */ }; struct expr *expr_parse__(struct expr_context *); @@ -1077,11 +1080,18 @@ expr_parse_primary(struct expr_context *ctx, bool *atomic) { *atomic = false; if (lexer_match(ctx->lexer, LEX_T_LPAREN)) { + if (++ctx->paren_depth > MAX_PAREN_DEPTH) { + lexer_syntax_error(ctx->lexer, + "parenthesis nested too deeply"); + return NULL; + } + struct expr *e = expr_parse__(ctx); if (!lexer_force_match(ctx->lexer, LEX_T_RPAREN)) { expr_destroy(e); return NULL; } + --ctx->paren_depth; *atomic = true; return e; } diff --git a/tests/ovn.at b/tests/ovn.at index 44475175d20a..39122b08bf22 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -285,6 +285,8 @@ ip6.src == ::1 => ip6.src == 0x1 inport == "eth0" !(inport != "eth0") => inport == "eth0" +(((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((0))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))) => 0 + ip4.src == "eth0" => Integer field ip4.src is not compatible with string constant. inport == 1 => String field inport is not compatible with integer constant. ip4.src = 1.2.3.4 => Syntax error at `=' expecting relational operator. @@ -351,6 +353,8 @@ eth.dst[40] x => Syntax error at `x' expecting end of input. ip4.src == {1.2.3.4, $set1, $unknownset} => Syntax error at `$unknownset' expecting address set name. eth.src == {$set3, badmac, 00:00:00:00:00:01} => Syntax error at `badmac' expecting constant. + +((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((( => Syntax error at end of input parenthesis nested too deeply. ]]) sed 's/ =>.*//' test-cases.txt > input.txt sed 's/.* => //' test-cases.txt > expout
This patch checks the depth of nested parentheses to prevent stack overflow. Since is_chassis_resident doesn't allow nested parentheses, its following parentheses are not taken into acount in the parentheses-depth context. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10714 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> Suggested-by: Ben Pfaff <blp@ovn.org> --- v1->v2: Handle parse_chassis_resident and add new test, thanks Ben! v2->v3: Ignore parentheses from chassis resident. ovn/lib/expr.c | 10 ++++++++++ tests/ovn.at | 4 ++++ 2 files changed, 14 insertions(+)