[nft] parser_json: fix segfault in translating string to nft object
diff mbox series

Message ID 20190411085940.l47vszzffm4e3e3c@nevthink
State Accepted
Headers show
Series
  • [nft] parser_json: fix segfault in translating string to nft object
Related show

Commit Message

Laura Garcia April 11, 2019, 8:59 a.m. UTC
A segmentation fault is produced when applying an input JSON file
like the following:

{"nftables": [
	{ "add":
		{"map":
			{"family": "ip",
				"name": "persistencia",
				"table": "nftlb",
				"type": "ipv4_addr",
				"map": "mark",
				"size": 65535,
				"flags": ["timeout"],
				"timeout": 44
			}
		}
	}
]}

The captured error is:

 Program received signal SIGSEGV, Segmentation fault.
 #1  0x00007ffff7f734f9 in string_to_nft_object (str=0x55555555f410
  "mark") at parser_json.c:2513
 2513			if (!strcmp(str, obj_tbl[i]))

The obj_tbl array is allocated with the maximum element index even
if lower indexes are not populated, so it produces null pointer
items.

This patch ensures that the maximum number of possible indexes
but also the element is not comparing a null pointer.

Signed-off-by: Laura Garcia Liebana <nevola@gmail.com>
---
 src/parser_json.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Florian Westphal April 11, 2019, 9:15 a.m. UTC | #1
Laura Garcia Liebana <nevola@gmail.com> wrote:
> The obj_tbl array is allocated with the maximum element index even
> if lower indexes are not populated, so it produces null pointer
> items.
> 
> This patch ensures that the maximum number of possible indexes
> but also the element is not comparing a null pointer.

Applied, thanks Laura.

>  static int string_to_nft_object(const char *str)
>  {
> -	const char *obj_tbl[] = {
> +	const char *obj_tbl[__NFT_OBJECT_MAX] = {
>  		[NFT_OBJECT_COUNTER] = "counter",
>  		[NFT_OBJECT_QUOTA] = "quota",
>  		[NFT_OBJECT_CT_HELPER] = "ct helper",
>  		[NFT_OBJECT_LIMIT] = "limit",
>  		[NFT_OBJECT_SECMARK] = "secmark",
>  	};

Phil, does this need updating?

It looks to me as if this should also init NFT_OBJECT_CT_TIMEOUT and so on.
Phil Sutter April 11, 2019, 10:34 a.m. UTC | #2
Hi,

On Thu, Apr 11, 2019 at 11:15:58AM +0200, Florian Westphal wrote:
> Laura Garcia Liebana <nevola@gmail.com> wrote:
> > The obj_tbl array is allocated with the maximum element index even
> > if lower indexes are not populated, so it produces null pointer
> > items.
> > 
> > This patch ensures that the maximum number of possible indexes
> > but also the element is not comparing a null pointer.
> 
> Applied, thanks Laura.

Thanks for catching this, Laura!

> >  static int string_to_nft_object(const char *str)
> >  {
> > -	const char *obj_tbl[] = {
> > +	const char *obj_tbl[__NFT_OBJECT_MAX] = {
> >  		[NFT_OBJECT_COUNTER] = "counter",
> >  		[NFT_OBJECT_QUOTA] = "quota",
> >  		[NFT_OBJECT_CT_HELPER] = "ct helper",
> >  		[NFT_OBJECT_LIMIT] = "limit",
> >  		[NFT_OBJECT_SECMARK] = "secmark",
> >  	};
> 
> Phil, does this need updating?
> 
> It looks to me as if this should also init NFT_OBJECT_CT_TIMEOUT and so on.

Actually, it is not strict enough. The function is used when handling
'add map' command. In bison, only counter, quota, limit and secmark are
allowed as stateful object "destination". I suspect ct helper is a
leftover from reusing the function somewhere else. I'll send a patch to
remove it.

Cheers, Phil

Patch
diff mbox series

diff --git a/src/parser_json.c b/src/parser_json.c
index 827604b..d0eacb6 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -2500,17 +2500,18 @@  static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
 
 static int string_to_nft_object(const char *str)
 {
-	const char *obj_tbl[] = {
+	const char *obj_tbl[__NFT_OBJECT_MAX] = {
 		[NFT_OBJECT_COUNTER] = "counter",
 		[NFT_OBJECT_QUOTA] = "quota",
 		[NFT_OBJECT_CT_HELPER] = "ct helper",
 		[NFT_OBJECT_LIMIT] = "limit",
 		[NFT_OBJECT_SECMARK] = "secmark",
 	};
+
 	unsigned int i;
 
-	for (i = 1; i < array_size(obj_tbl); i++) {
-		if (!strcmp(str, obj_tbl[i]))
+	for (i = 0; i < NFT_OBJECT_MAX; i++) {
+		if (obj_tbl[i] && !strcmp(str, obj_tbl[i]))
 			return i;
 	}
 	return 0;