On 11/1/20 10:15 PM, Simon Glass wrote:
Add tests to check for buffer overflow using simple replacement as well as back references. At present these don't fully pass.
Signed-off-by: Simon Glass sjg@chromium.org
cmd/setexpr.c | 21 +++-------- include/command.h | 17 +++++++++ test/cmd/setexpr.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 17 deletions(-)
diff --git a/cmd/setexpr.c b/cmd/setexpr.c index fe3435b4d99..dbb43b3be2f 100644 --- a/cmd/setexpr.c +++ b/cmd/setexpr.c @@ -134,22 +134,8 @@ static char *substitute(char *string, int *slen, int ssize, return p + nlen; }
-/**
- regex_sub() - Replace a regex pattern with a string
- @data: Buffer containing the string to update
- @data_size: Size of buffer (must be large enough for the new string)
- @nbuf: Back-reference buffer
- @nbuf_size: Size of back-reference buffer (must be larger enough for @s plus
- all back-reference expansions)
- @r: Regular expression to find
- @s: String to replace with
- @global: true to replace all matches in @data, false to replace just the
- first
- @return 0 if OK, 1 on error
- */
-static int regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
const char *r, const char *s, bool global)+int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
const char *r, const char *s, bool global)@@ -325,7 +311,8 @@ static int regex_sub_var(const char *name, const char *r, const char *s,
strcpy(data, t);
- ret = regex_sub(data, SLRE_BUFSZ, nbuf, SLRE_PATSZ, r, s, global);
- ret = setexpr_regex_sub(data, SLRE_BUFSZ, nbuf, SLRE_PATSZ, r, s,
global);diff --git a/include/command.h b/include/command.h index e900f97df33..e229bf2825c 100644 --- a/include/command.h +++ b/include/command.h @@ -183,6 +183,23 @@ extern int do_env_set_efi(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); #endif
+/**
- setexpr_regex_sub() - Replace a regex pattern with a string
- @data: Buffer containing the string to update
- @data_size: Size of buffer (must be large enough for the new string)
- @nbuf: Back-reference buffer
- @nbuf_size: Size of back-reference buffer (must be larger enough for @s plus
- all back-reference expansions)
- @r: Regular expression to find
- @s: String to replace with
- @global: true to replace all matches in @data, false to replace just the
- first
- @return 0 if OK, 1 on error
- */
+int setexpr_regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
const char *r, const char *s, bool global);- /*
- Error codes that commands return to cmd_process(). We use the standard 0
- and 1 for success and failure, but add one more case - failure with a
diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c index de54561917c..a6940fd82dd 100644 --- a/test/cmd/setexpr.c +++ b/test/cmd/setexpr.c @@ -209,6 +209,95 @@ static int setexpr_test_regex_inc(struct unit_test_state *uts) } SETEXPR_TEST(setexpr_test_regex_inc, UT_TESTF_CONSOLE_REC);
+/* Test setexpr_regex_sub() directly to check buffer usage */ +static int setexpr_test_sub(struct unit_test_state *uts) +{
- char *buf, *nbuf;
- int i;
- buf = map_sysmem(0, BUF_SIZE);
- nbuf = map_sysmem(0x1000, BUF_SIZE);
- /* Add a pattern so we can check the buffer limits */
- memset(buf, '\xff', BUF_SIZE);
- memset(nbuf, '\xff', BUF_SIZE);
- for (i = BUF_SIZE; i < 0x1000; i++) {
buf[i] = i & 0xff;nbuf[i] = i & 0xff;- }
- strcpy(buf, "this is a test");
- /*
* This is a regression test, since a bug was found in the use of* memmove() in setexpr*/- 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",i, i & 0xff, (u8)buf[i]);ut_assertf(nbuf[i] == (char)i,"nbuf byte at %x should be %02x, got %02x)\n",i, i & 0xff, (u8)nbuf[i]);- }
- unmap_sysmem(buf);
- return 0;
+} +SETEXPR_TEST(setexpr_test_sub, UT_TESTF_CONSOLE_REC);
+/* Test setexpr_regex_sub() with back references */ +static int setexpr_test_backref(struct unit_test_state *uts) +{
- char *buf, *nbuf;
- int i;
- buf = map_sysmem(0, BUF_SIZE);
- nbuf = map_sysmem(0x1000, BUF_SIZE);
This test is compiled for all boards enabling CONFIG_UNIT_TEST and CONFIG_CMDLINE.
On the sipeed_maix_smode_defconfig trying to access NULL results in a crash:
Test: setexpr_test_backref Unhandled exception: Store/AMO access fault EPC: 00000000805b1152 RA: 00000000805b3bce TVAL: 0000000000001000 EPC: 0000000080056152 RA: 0000000080058bce reloc adjusted
Why did you opt for hard-coded addresses instead of malloc()?
We should be able to run the C unit tests on all boards not only on the sandbox except where sandbox drivers are involved.
Could you, please, provide a correction.
Best regards
Heinrich
- /* Add a pattern so we can check the buffer limits */
- memset(buf, '\xff', BUF_SIZE);
- memset(nbuf, '\xff', BUF_SIZE);
- for (i = BUF_SIZE; i < 0x1000; i++) {
buf[i] = i & 0xff;nbuf[i] = i & 0xff;- }
- strcpy(buf, "this is surely a test is it? yes this is indeed a test");
- /*
* This is a regression test, since a bug was found in the use of* memmove() in setexpr*/- ut_assertok(setexpr_regex_sub(buf, BUF_SIZE, nbuf, BUF_SIZE,
"(this) (is) (surely|indeed)","us \\1 \\2 \\3!", true));- /* The following checks fail at present due to bugs in setexpr */
- return 0;
- ut_asserteq_str("us this is surely! a test is it? yes us this is indeed! a test",
buf);- for (i = BUF_SIZE; i < 0x1000; i++) {
ut_assertf(buf[i] == (char)i,"buf byte at %x should be %02x, got %02x)\n",i, i & 0xff, (u8)buf[i]);ut_assertf(nbuf[i] == (char)i,"nbuf byte at %x should be %02x, got %02x)\n",i, i & 0xff, (u8)nbuf[i]);- }
- unmap_sysmem(buf);
- return 0;
+} +SETEXPR_TEST(setexpr_test_backref, UT_TESTF_CONSOLE_REC);
- int do_ut_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { struct unit_test *tests = ll_entry_start(struct unit_test,