Hello,

Here it is, I've introduced a bug in last patch (since it is recent I've put "MINOR", even if it might have bigger impact in prod.

Also, a patch for better error diagnostics, and to report any extra data in the filter.

Best regards,
--
Adis Nezirovic
Software Engineer
HAProxy Technologies - Powering your uptime!
375 Totten Pond Road, Suite 302 | Waltham, MA 02451, US
+1 (844) 222-4340 | https://www.haproxy.com
>From 030ee62e99d0ede6ea2e0bae16d4621cdd77da19 Mon Sep 17 00:00:00 2001
From: Adis Nezirovic <[email protected]>
Date: Wed, 22 Jan 2020 16:16:48 +0100
Subject: [PATCH 1/2] BUG/MINOR: cli: Missing arg offset for filter data
 values.

We don't properly check for missing data values for additional filter
entries, passing out of bounds index to args[], then passing to strlen.

Introduced in commit 1a693fc2: (MEDIUM: cli: Allow multiple filter
entries for "show table")
---
 src/stick_table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/stick_table.c b/src/stick_table.c
index 1393b1ff3..beec279c2 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -3620,7 +3620,7 @@ static int table_prepare_data_request(struct appctx *appctx, char **args)
 		if (appctx->ctx.table.data_op < 0)
 			return cli_err(appctx, "Require and operator among \"eq\", \"ne\", \"le\", \"ge\", \"lt\", \"gt\"\n");
 
-		if (!*args[5] || strl2llrc(args[5+3*i], strlen(args[5+3*i]), &appctx->ctx.table.value[i]) != 0)
+		if (!*args[5+3*i] || strl2llrc(args[5+3*i], strlen(args[5+3*i]), &appctx->ctx.table.value[i]) != 0)
 			return cli_err(appctx, "Require a valid integer value to compare against\n");
 	}
 
-- 
2.25.0

>From d193f62ff73f01deb1eae77c0d3f319af50858ad Mon Sep 17 00:00:00 2001
From: Adis Nezirovic <[email protected]>
Date: Wed, 22 Jan 2020 16:50:27 +0100
Subject: [PATCH 2/2] MINOR: cli: Report location of error or any extra data
 for "show table"

---
 src/stick_table.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/stick_table.c b/src/stick_table.c
index beec279c2..b7d5d145b 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -3601,6 +3601,7 @@ static int table_process_entry_per_key(struct appctx *appctx, char **args)
 static int table_prepare_data_request(struct appctx *appctx, char **args)
 {
 	int i;
+	char *err = NULL;
 
 	if (appctx->ctx.table.action != STK_CLI_ACT_SHOW && appctx->ctx.table.action != STK_CLI_ACT_CLR)
 		return cli_err(appctx, "content-based lookup is only supported with the \"show\" and \"clear\" actions\n");
@@ -3611,17 +3612,21 @@ static int table_prepare_data_request(struct appctx *appctx, char **args)
 		/* condition on stored data value */
 		appctx->ctx.table.data_type[i] = stktable_get_data_type(args[3+3*i] + 5);
 		if (appctx->ctx.table.data_type[i] < 0)
-			return cli_err(appctx, "Unknown data type\n");
+			return cli_dynerr(appctx, memprintf(&err, "Filter entry #%i: Unknown data type\n", i + 1));
 
 		if (!((struct stktable *)appctx->ctx.table.target)->data_ofs[appctx->ctx.table.data_type[i]])
-			return cli_err(appctx, "Data type not stored in this table\n");
+			return cli_dynerr(appctx, memprintf(&err, "Filter entry #%i: Data type not stored in this table\n", i + 1));
 
 		appctx->ctx.table.data_op[i] = get_std_op(args[4+3*i]);
 		if (appctx->ctx.table.data_op < 0)
-			return cli_err(appctx, "Require and operator among \"eq\", \"ne\", \"le\", \"ge\", \"lt\", \"gt\"\n");
+			return cli_dynerr(appctx, memprintf(&err, "Filter entry #%i: Require and operator among \"eq\", \"ne\", \"le\", \"ge\", \"lt\", \"gt\"\n", i + 1));
 
 		if (!*args[5+3*i] || strl2llrc(args[5+3*i], strlen(args[5+3*i]), &appctx->ctx.table.value[i]) != 0)
-			return cli_err(appctx, "Require a valid integer value to compare against\n");
+			return cli_dynerr(appctx, memprintf(&err, "Filter entry #%i: Require a valid integer value to compare against\n", i + 1));
+	}
+
+	if (*args[3+3*i]) {
+		return cli_dynerr(appctx, memprintf(&err, "Detected extra data in filter, %ith word of input, after '%s'\n", 3+3*i + 1, args[2+3*i]));
 	}
 
 	/* OK we're done, all the fields are set */
-- 
2.25.0

Reply via email to