diff mbox series

[08/10] setexpr: Correct buffer overflow bug and enable tests

Message ID 20201101211544.3579850-9-sjg@chromium.org
State Accepted
Commit 8f4aa7ddb908369db971d4c31850ca1eef2e3687
Delegated to: Tom Rini
Headers show
Series setexpr: Correct various bugs and add tests plus string support | expand

Commit Message

Simon Glass Nov. 1, 2020, 9:15 p.m. UTC
At present when more than one substitution is made this function
overwrites its buffers. Fix this bug and update the tests now that they
can pass.

Also update the debug code to show all substrings, since at present it
omits the final one.

Fixes: 855f18ea0e6 ("setexpr: add regex substring matching and substitution")
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/setexpr.c      | 10 +++++-----
 test/cmd/setexpr.c | 24 +++++++-----------------
 2 files changed, 12 insertions(+), 22 deletions(-)

Comments

Tom Rini Dec. 2, 2020, 9:23 p.m. UTC | #1
On Sun, Nov 01, 2020 at 02:15:42PM -0700, Simon Glass wrote:

> At present when more than one substitution is made this function
> overwrites its buffers. Fix this bug and update the tests now that they
> can pass.
> 
> Also update the debug code to show all substrings, since at present it
> omits the final one.
> 
> Fixes: 855f18ea0e6 ("setexpr: add regex substring matching and substitution")
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!
diff mbox series

Patch

diff --git a/cmd/setexpr.c b/cmd/setexpr.c
index 0cc7cf15bd7..d364dbc2bc5 100644
--- a/cmd/setexpr.c
+++ b/cmd/setexpr.c
@@ -155,11 +155,11 @@  int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
 
 		(void) memset(caps, 0, sizeof(caps));
 
-		res = slre_match(&slre, datap, len, caps);
+		res = slre_match(&slre, datap, len - (datap - data), caps);
 
 		debug("Result: %d\n", res);
 
-		for (i = 0; i < slre.num_caps; i++) {
+		for (i = 0; i <= slre.num_caps; i++) {
 			if (caps[i].len > 0) {
 				debug("Substring %d: [%.*s]\n", i,
 					caps[i].len, caps[i].ptr);
@@ -231,7 +231,7 @@  int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
 					break;
 
 				np = substitute(np, &nlen,
-					nbuf_size,
+					nbuf_size - (np - nbuf),
 					backref, 2,
 					caps[i].ptr, caps[i].len);
 
@@ -241,8 +241,8 @@  int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
 		}
 		debug("## SUBST(2) ## %s\n", nbuf);
 
-		datap = substitute(datap, &len, data_size, old, olen,
-				   nbuf, nlen);
+		datap = substitute(datap, &len, data_size - (datap - data),
+				   old, olen, nbuf, nlen);
 
 		if (datap == NULL)
 			return 1;
diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c
index d06dda260e6..2a897efd9bd 100644
--- a/test/cmd/setexpr.c
+++ b/test/cmd/setexpr.c
@@ -166,12 +166,10 @@  static int setexpr_test_regex(struct unit_test_state *uts)
 
 	/* Global substitution */
 	ut_assertok(run_command("setenv fred 'this is a test'", 0));
-	if (0) {
-		/* Causes a crash at present due to a bug in setexpr */
-		ut_assertok(run_command("setexpr fred gsub is us", 0));
-		val = env_get("fred");
-		ut_asserteq_str("thus us a test", val);
-	}
+	ut_assertok(run_command("setexpr fred gsub is us", 0));
+	val = env_get("fred");
+	ut_asserteq_str("thus us a test", val);
+
 	/* Global substitution */
 	ut_assertok(run_command("setenv fred 'this is a test'", 0));
 	ut_assertok(run_command("setenv mary 'this is a test'", 0));
@@ -195,14 +193,9 @@  static int setexpr_test_regex_inc(struct unit_test_state *uts)
 	buf = map_sysmem(0, BUF_SIZE);
 
 	ut_assertok(run_command("setenv fred 'this is a test'", 0));
-	if (0) {
-		/* Causes a crash at present due to a bug in setexpr */
-		ut_assertok(run_command("setexpr fred gsub is much_longer_string",
-					0));
-		val = env_get("fred");
-		ut_asserteq_str("thmuch_longer_string much_longer_string a test",
-				val);
-	}
+	ut_assertok(run_command("setexpr fred gsub is much_longer_string", 0));
+	val = env_get("fred");
+	ut_asserteq_str("thmuch_longer_string much_longer_string a test", val);
 	unmap_sysmem(buf);
 
 	return 0;
@@ -234,9 +227,6 @@  static int setexpr_test_sub(struct unit_test_state *uts)
 	ut_assertok(setexpr_regex_sub(buf, BUF_SIZE, nbuf, BUF_SIZE, "is",
 				      "us it is longer", true));
 	ut_asserteq_str("thus it is longer us it is longer a test", buf);
-
-	/* The following checks fail at present due to a bug in setexpr */
-	return 0;
 	for (i = BUF_SIZE; i < 0x1000; i++) {
 		ut_assertf(buf[i] == (char)i,
 			   "buf byte at %x should be %02x, got %02x)\n",