ofp-actions: Properly check for action that exceeds buffer length.

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@Contextream.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Justin Pettit <jpettit@nicira.com>

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 7d9ee585ab7c76c788b03302357578fe64137018..41c76226d21ea91b3a7bb030ecb3087abb2d4583 100644 (file)
--- 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 64b4bc2bc10d75916cc5844eec292797f003614d..af5dd19da6275dafe83bcdccbbd2e0205f76b3ad 100644 (file)
--- 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 not in valid range [[8,8]]
+& ofp_actions|WARN|bad action at offset 0 (OFPBAC_BAD_LEN):
+& 00000000  00 00 00 10 ff fe ff ff-00 00 00 00 00 00 00 00
+00 00 00 10 ff fe ff ff 00 00 00 00 00 00 00 00
+
+# bad OpenFlow10 actions: OFPBAC_BAD_LEN
+& ofp_actions|WARN|OpenFlow action NXAST_DEC_TTL_CNT_IDS length 17 is not a multiple of 8
+ffff 0011 00002320 0015 0001 00000000 0000000000000000
+
 ])
 sed '/^[[#&]]/d' < test-data > input.txt
 sed -n 's/^# //p; /^$/p' < test-data > expout