[ovs-dev] [PATCH] ofp-actions: Properly check for action that exceeds buffer length.

Ben Pfaff blp at nicira.com
Mon Oct 20 21:45:42 UTC 2014


Commit c2d936a44fa (ofp-actions: Centralize all OpenFlow action code for
maintainability.) rewrote OpenFlow action parsing but failed to check that
actions don't overflow their buffers.  This commit fixes the problem and
adds negative tests so that this bug doesn't recur.

Reported-by: Tomer Pearl <Tomer.Pearl at Contextream.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/ofp-actions.c    |    5 +++++
 tests/ofp-actions.at |   16 ++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 7d9ee58..41c7622 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -6406,6 +6406,11 @@ ofpact_pull_raw(struct ofpbuf *buf, enum ofp_version ofp_version,
     }
 
     length = ntohs(oah->len);
+    if (length > ofpbuf_size(buf)) {
+        VLOG_WARN_RL(&rl, "OpenFlow action %s length %u exceeds action buffer "
+                     "length %"PRIu32, action->name, length, ofpbuf_size(buf));
+        return OFPERR_OFPBAC_BAD_LEN;
+    }
     if (length < action->min_length || length > action->max_length) {
         VLOG_WARN_RL(&rl, "OpenFlow action %s length %u not in valid range "
                      "[%hu,%hu]", action->name, length,
diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
index 64b4bc2..311c3c5 100644
--- a/tests/ofp-actions.at
+++ b/tests/ofp-actions.at
@@ -119,6 +119,22 @@ ffff 0020 00002320 0015 000500000000 80003039005A02fd 0400000000000000
 # actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
 ffff 0018 00002320 001d 3039 00005BA0 00008707 0000B26E
 
+# bad OpenFlow10 actions: OFPBAC_BAD_LEN
+& ofp_actions|WARN|OpenFlow action OFPAT_OUTPUT length 240 exceeds action buffer length 8
+& ofp_actions|WARN|bad action at offset 0 (OFPBAC_BAD_LEN):
+& 00000000  00 00 00 f0 00 00 00 00-
+00 00 00 f0 00 00 00 00
+
+# bad OpenFlow10 actions: OFPBAC_BAD_LEN
+& ofp_actions|WARN|OpenFlow action OFPAT_OUTPUT length 16 exceeds action buffer length 8
+& ofp_actions|WARN|bad action at offset 0 (OFPBAC_BAD_LEN):
+& 00000000  00 00 00 10 ff fe ff ff-
+00 00 00 10 ff fe ff ff
+
+# bad OpenFlow10 actions: OFPBAC_BAD_LEN
+& ofp_actions|WARN|OpenFlow action OFPAT_OUTPUT length 9 exceeds action buffer length 8
+00 00 00 09 ff fe ff ff
+
 ])
 sed '/^[[#&]]/d' < test-data > input.txt
 sed -n 's/^# //p; /^$/p' < test-data > expout
-- 
1.7.10.4




More information about the dev mailing list