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

[Justin Pettit]

Acked-by: Justin Pettit <jpettit at nicira.com>

--Justin


> On Oct 20, 2014, at 2:45 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> 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
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev