[ovs-dev] [PATCH] ofp-actions: Report an error if there are too many actions to parse.

Ilya Maximets i.maximets at ovn.org
Wed Jun 16 20:34:48 UTC 2021


Not a very important fix, but fuzzer times out trying to test parsing
of a huge number of actions.  Fixing that by reporting an error as
soon as ofpacts oversized.

It would be great to use ofpacts_oversized() function instead of manual
size checking, but ofpacts->header here always points to the last
pushed action, so the value that ofpacts_oversized() would check is
always small.

Adding a unit test for this, plus the extra test for too deep nesting.

Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20254
Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
---
 lib/ofp-actions.c    | 4 ++++
 tests/ofp-actions.at | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 6fb3da507..ecf914eac 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -9326,6 +9326,7 @@ static char * OVS_WARN_UNUSED_RESULT
 ofpacts_parse__(char *str, const struct ofpact_parse_params *pp,
                 bool allow_instructions, enum ofpact_type outer_action)
 {
+    uint32_t orig_size = pp->ofpacts->size;
     char *key, *value;
     bool drop = false;
     char *pos;
@@ -9370,6 +9371,9 @@ ofpacts_parse__(char *str, const struct ofpact_parse_params *pp,
         if (error) {
             return error;
         }
+        if (pp->ofpacts->size - orig_size > UINT16_MAX) {
+            return xasprintf("input too big");
+        }
     }
 
     if (drop && pp->ofpacts->size) {
diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
index 59093c03c..ef4898abb 100644
--- a/tests/ofp-actions.at
+++ b/tests/ofp-actions.at
@@ -1144,4 +1144,10 @@ bad_action 'apply_actions' 'apply_actions is the default instruction'
 bad_action 'xyzzy' 'unknown action xyzzy'
 bad_action 'drop,3' '"drop" must not be accompanied by any other action or instruction'
 
+# Too many actions
+writes=$(printf 'write_actions(%.0s' $(seq 100))
+bad_action "${writes}" 'Action nested too deeply'
+outputs=$(printf '1,%.0s' $(seq 4096))
+bad_action "${outputs}" 'input too big'
+
 AT_CLEANUP
-- 
2.26.3



More information about the dev mailing list