From 8e58e96075dea109d217ba7070043e0d5d574b66 Mon Sep 17 00:00:00 2001
From: "Jose M. Guisado Gomez" <guigom@riseup.net>
Date: Thu, 10 Sep 2020 18:40:20 +0200
Subject: parser_bison: fail when specifying multiple comments

Before this patch grammar supported specifying multiple comments, and
only the last value would be assigned.

This patch adds a function to test if an attribute is already assigned
and, if so, calls erec_queue with this attribute location.

Use this function in order to check for duplication (or more) of comments
for actions that support it.

> nft add table inet filter { flags "dormant"\; comment "test"\; comment "another"\;}

Error: You can only specify this once. This statement is duplicated.
add table inet filter { flags dormant; comment test; comment another;}
                                                     ^^^^^^^^^^^^^^^^

Signed-off-by: Jose M. Guisado Gomez <guigom@riseup.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/parser_bison.y | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

(limited to 'src/parser_bison.y')

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 7242c4c3..524903fd 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -121,6 +121,17 @@ static struct expr *handle_concat_expr(const struct location *loc,
 	return expr;
 }
 
+static bool already_set(const void *attr, const struct location *loc,
+			struct parser_state *state)
+{
+	if (!attr)
+		return false;
+
+	erec_queue(error(loc, "You can only specify this once. This statement is duplicated."),
+		   state->msgs);
+	return true;
+}
+
 #define YYLLOC_DEFAULT(Current, Rhs, N)	location_update(&Current, Rhs, N)
 
 #define symbol_value(loc, str) \
@@ -1556,6 +1567,10 @@ table_options		:	FLAGS		STRING
 			}
 			|	comment_spec
 			{
+				if (already_set($<table>0->comment, &@$, state)) {
+					xfree($1);
+					YYERROR;
+				}
 				$<table>0->comment = $1;
 			}
 			;
@@ -1795,6 +1810,10 @@ set_block		:	/* empty */	{ $$ = $<set>-1; }
 			|	set_block	set_mechanism	stmt_separator
 			|	set_block	comment_spec	stmt_separator
 			{
+				if (already_set($1->comment, &@2, state)) {
+					xfree($2);
+					YYERROR;
+				}
 				$1->comment = $2;
 				$$ = $1;
 			}
@@ -1923,6 +1942,10 @@ map_block		:	/* empty */	{ $$ = $<set>-1; }
 			}
 			|	map_block	comment_spec	stmt_separator
 			{
+				if (already_set($1->comment, &@2, state)) {
+					xfree($2);
+					YYERROR;
+				}
 				$1->comment = $2;
 				$$ = $1;
 			}
@@ -2061,6 +2084,10 @@ counter_block		:	/* empty */	{ $$ = $<obj>-1; }
 			}
 			|	counter_block	  comment_spec
 			{
+				if (already_set($<obj>1->comment, &@2, state)) {
+					xfree($2);
+					YYERROR;
+				}
 				$<obj>1->comment = $2;
 			}
 			;
@@ -2074,6 +2101,10 @@ quota_block		:	/* empty */	{ $$ = $<obj>-1; }
 			}
 			|	quota_block	comment_spec
 			{
+				if (already_set($<obj>1->comment, &@2, state)) {
+					xfree($2);
+					YYERROR;
+				}
 				$<obj>1->comment = $2;
 			}
 			;
@@ -2087,6 +2118,10 @@ ct_helper_block		:	/* empty */	{ $$ = $<obj>-1; }
 			}
 			|       ct_helper_block     comment_spec
 			{
+				if (already_set($<obj>1->comment, &@2, state)) {
+					xfree($2);
+					YYERROR;
+				}
 				$<obj>1->comment = $2;
 			}
 			;
@@ -2104,6 +2139,10 @@ ct_timeout_block	:	/*empty */
 			}
 			|       ct_timeout_block     comment_spec
 			{
+				if (already_set($<obj>1->comment, &@2, state)) {
+					xfree($2);
+					YYERROR;
+				}
 				$<obj>1->comment = $2;
 			}
 			;
@@ -2117,6 +2156,10 @@ ct_expect_block		:	/*empty */	{ $$ = $<obj>-1; }
 			}
 			|       ct_expect_block     comment_spec
 			{
+				if (already_set($<obj>1->comment, &@2, state)) {
+					xfree($2);
+					YYERROR;
+				}
 				$<obj>1->comment = $2;
 			}
 			;
@@ -2130,6 +2173,10 @@ limit_block		:	/* empty */	{ $$ = $<obj>-1; }
 			}
 			|       limit_block     comment_spec
 			{
+				if (already_set($<obj>1->comment, &@2, state)) {
+					xfree($2);
+					YYERROR;
+				}
 				$<obj>1->comment = $2;
 			}
 			;
@@ -2143,6 +2190,10 @@ secmark_block		:	/* empty */	{ $$ = $<obj>-1; }
 			}
 			|       secmark_block     comment_spec
 			{
+				if (already_set($<obj>1->comment, &@2, state)) {
+					xfree($2);
+					YYERROR;
+				}
 				$<obj>1->comment = $2;
 			}
 			;
@@ -2156,6 +2207,10 @@ synproxy_block		:	/* empty */	{ $$ = $<obj>-1; }
 			}
 			|       synproxy_block     comment_spec
 			{
+				if (already_set($<obj>1->comment, &@2, state)) {
+					xfree($2);
+					YYERROR;
+				}
 				$<obj>1->comment = $2;
 			}
 			;
@@ -4000,6 +4055,10 @@ set_elem_option		:	TIMEOUT			time_spec
 			}
 			|	comment_spec
 			{
+				if (already_set($<expr>0->comment, &@1, state)) {
+					xfree($1);
+					YYERROR;
+				}
 				$<expr>0->comment = $1;
 			}
 			;
@@ -4034,6 +4093,10 @@ set_elem_expr_option	:	TIMEOUT			time_spec
 			}
 			|	comment_spec
 			{
+				if (already_set($<expr>0->comment, &@1, state)) {
+					xfree($1);
+					YYERROR;
+				}
 				$<expr>0->comment = $1;
 			}
 			;
-- 
cgit v1.2.3