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

[Ilya Maximets]

On 6/16/21 10:34 PM, Ilya Maximets wrote:
> 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.

Ugh.  s/ofpacts_oversized/ofpbuf_oversized/
I can fix that on commit if the new version will not be necessary
otherwise.

> 
> 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
>