From b153b9903272f858ee1bbaaba0768f5f2d6506e7 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 23 Oct 2014 14:34:04 -0700 Subject: [PATCH] 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 Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- 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 7d9ee585a..41c76226d 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 64b4bc2bc..af5dd19da 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 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