diff mbox series

[ovs-dev,v2] expr: Set a limit on the depth of nested parentheses

Message ID 1539194634-348-1-git-send-email-pkusunyifeng@gmail.com
State Superseded
Headers show
Series [ovs-dev,v2] expr: Set a limit on the depth of nested parentheses | expand

Commit Message

Yifeng Sun Oct. 10, 2018, 6:03 p.m. UTC 
This patch checks the depth of nested parentheses to prevent
stack overflow.

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!

 ovn/lib/expr.c | 17 +++++++++++++++++
 tests/ovn.at   |  5 +++++
 2 files changed, 22 insertions(+)

Comments

Ben Pfaff Oct. 10, 2018, 6:13 p.m. UTC | #1 
On Wed, Oct 10, 2018 at 11:03:54AM -0700, Yifeng Sun wrote:
> This patch checks the depth of nested parentheses to prevent
> stack overflow.
> 
> 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!

Thanks for the new version.

I don't understand why is_chassis_resident() is a special case.  Can you
help me understand?

Thanks,

Ben.
Yifeng Sun Oct. 10, 2018, 10:16 p.m. UTC | #2 
Thanks, handling is_chassis_resident is not necessary because it doesn't
allow nested parentheses. I've sent out a new version.

Yifeng


On Wed, Oct 10, 2018 at 11:13 AM Ben Pfaff <blp@ovn.org> wrote:

> On Wed, Oct 10, 2018 at 11:03:54AM -0700, Yifeng Sun wrote:
> > This patch checks the depth of nested parentheses to prevent
> > stack overflow.
> >
> > 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!
>
> Thanks for the new version.
>
> I don't understand why is_chassis_resident() is a special case.  Can you
> help me understand?
>
> Thanks,
>
> Ben.
>
diff mbox series

Patch

diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 148ac869e861..96e61606e288 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 *);
@@ -1068,6 +1071,7 @@  parse_chassis_resident(struct expr_context *ctx)
         expr_destroy(e);
         return NULL;
     }
+    --ctx->paren_depth;
 
     return e;
 }
@@ -1077,11 +1081,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;
     }
@@ -1093,6 +1104,12 @@  expr_parse_primary(struct expr_context *ctx, bool *atomic)
 
         if (lexer_lookahead(ctx->lexer) == LEX_T_LPAREN) {
             if (lexer_match_id(ctx->lexer, "is_chassis_resident")) {
+                if (++ctx->paren_depth > MAX_PAREN_DEPTH) {
+                    lexer_syntax_error(ctx->lexer,
+                                       "parenthesis nested too deeply");
+                    return NULL;
+                }
+
                 lexer_get(ctx->lexer); /* Skip "(". */
                 *atomic = true;
                 return parse_chassis_resident(ctx);
diff --git a/tests/ovn.at b/tests/ovn.at
index 44475175d20a..584fb5c2a727 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"
 
+(((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((is_chassis_resident("eth0")))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))) => is_chassis_resident("eth0")
+
 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,9 @@  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.
+((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((is_chassis_resident("eth1") => Syntax error at `(' parenthesis nested too deeply.
 ]])
 sed 's/ =>.*//' test-cases.txt > input.txt
 sed 's/.* => //' test-cases.txt > expout