Re: [PATCH nft 5/5] datatype: check against negative "type" argument in datatype_lookup()

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, Aug 30, 2023 at 10:08:50AM +0200, Thomas Haller wrote:
[...]
> I don't think it suffices. The following fail the assertion (or would
> access out of bounds).
> 
> 
> diff --git c/include/datatype.h i/include/datatype.h
> index 9ce7359cd340..7d3b6b20d27c 100644
> --- c/include/datatype.h
> +++ i/include/datatype.h
> @@ -98,7 +98,8 @@ enum datatypes {
>      TYPE_TIME_HOUR,
>      TYPE_TIME_DAY,
>      TYPE_CGROUPV2,
> -    __TYPE_MAX
> +    __TYPE_MAX,
> +    __TYPE_FORCE_SIGNED = -1,

I don't expect to ever have a negative defined here.

>  };
>  #define TYPE_MAX        (__TYPE_MAX - 1)
>  
> diff --git c/src/datatype.c i/src/datatype.c
> index ba1192c83595..1ff8a4a08551 100644
> --- c/src/datatype.c
> +++ i/src/datatype.c
> @@ -89,6 +89,7 @@ const struct datatype *datatype_lookup(enum datatypes
> type)
>  
>      if (type > TYPE_MAX)
>           return NULL;
> +    assert(type != (enum datatypes) -1);
>      return datatypes[type];
>  }
>  
> diff --git c/src/libnftables.c i/src/libnftables.c
> index 9c802ec95f27..7e60d1a18d39 100644
> --- c/src/libnftables.c
> +++ i/src/libnftables.c
> @@ -203,6 +203,8 @@ struct nft_ctx *nft_ctx_new(uint32_t flags)
>  #endif
>      }
>  
> +    datatype_lookup(-1);
> +
>      ctx = xzalloc(sizeof(struct nft_ctx));
>      nft_init(ctx);
>  
> 
> 
> 
> If you expect that "type" is always valid, then there is no need to
> check against >TYPE_MAX. If you expect that it might be invalid, it
> seems prudent to also check against negative values.
> 
> 
> 
> Thomas
>