setexpr: Split the core logic into its own function
At present this function always allocates a large amount of stack, and
selects its own size for buffers. This makes it hard to test the code
for buffer overflow.
Separate out the inner logic of the substitution so that tests can call
this directly. This will allow checking that the algorithm does not
overflow the buffer.
Fix up one of the error lines at the same time, since it should be
printing nbuf_size, not data_size.
Signed-off-by: Simon Glass <sjg@chromium.org>
diff --git a/cmd/setexpr.c b/cmd/setexpr.c
index dd9c257..fe3435b 100644
--- a/cmd/setexpr.c
+++ b/cmd/setexpr.c
@@ -58,9 +58,6 @@
#include <slre.h>
-#define SLRE_BUFSZ 16384
-#define SLRE_PATSZ 4096
-
/*
* memstr - Find the first substring in memory
* @s1: The string to be searched
@@ -83,13 +80,24 @@
return NULL;
}
-static char *substitute(char *string, /* string buffer */
- int *slen, /* current string length */
- int ssize, /* string bufer size */
- const char *old,/* old (replaced) string */
- int olen, /* length of old string */
- const char *new,/* new (replacement) string */
- int nlen) /* length of new string */
+/**
+ * substitute() - Substitute part of one string with another
+ *
+ * This updates @string so that the first occurrence of @old is replaced with
+ * @new
+ *
+ * @string: String buffer containing string to update at the start
+ * @slen: Pointer to current string length, updated on success
+ * @ssize: Size of string buffer
+ * @old: Old string to find in the buffer (no terminator needed)
+ * @olen: Length of @old excluding terminator
+ * @new: New string to replace @old with
+ * @nlen: Length of @new excluding terminator
+ * @return pointer to immediately after the copied @new in @string, or NULL if
+ * no replacement took place
+ */
+static char *substitute(char *string, int *slen, int ssize,
+ const char *old, int olen, const char *new, int nlen)
{
char *p = memstr(string, *slen, old, olen);
@@ -118,7 +126,7 @@
memmove(p + nlen, p + olen, tail);
}
- /* insert substitue */
+ /* insert substitute */
memcpy(p, new, nlen);
*slen += nlen - olen;
@@ -126,52 +134,33 @@
return p + nlen;
}
-/*
- * Perform regex operations on a environment variable
+/**
+ * regex_sub() - Replace a regex pattern with a string
*
- * Returns 0 if OK, 1 in case of errors.
+ * @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(const char *name,
- const char *r, const char *s, const char *t,
- int global)
+static int regex_sub(char *data, uint data_size, char *nbuf, uint nbuf_size,
+ const char *r, const char *s, bool global)
{
struct slre slre;
- char data[SLRE_BUFSZ];
char *datap = data;
- const char *value;
int res, len, nlen, loop;
- if (name == NULL)
- return 1;
-
if (slre_compile(&slre, r) == 0) {
printf("Error compiling regex: %s\n", slre.err_str);
return 1;
}
- if (t == NULL) {
- value = env_get(name);
-
- if (value == NULL) {
- printf("## Error: variable \"%s\" not defined\n", name);
- return 1;
- }
- t = value;
- }
-
- debug("REGEX on %s=%s\n", name, t);
- debug("REGEX=\"%s\", SUBST=\"%s\", GLOBAL=%d\n",
- r, s ? s : "<NULL>", global);
-
- len = strlen(t);
- if (len + 1 > SLRE_BUFSZ) {
- printf("## error: subst buffer overflow: have %d, need %d\n",
- SLRE_BUFSZ, len + 1);
- return 1;
- }
-
- strcpy(data, t);
-
+ len = strlen(data);
if (s == NULL)
nlen = 0;
else
@@ -179,7 +168,6 @@
for (loop = 0;; loop++) {
struct cap caps[slre.num_caps + 2];
- char nbuf[SLRE_PATSZ];
const char *old;
char *np;
int i, olen;
@@ -199,7 +187,7 @@
if (res == 0) {
if (loop == 0) {
- printf("%s: No match\n", t);
+ printf("%s: No match\n", data);
return 1;
} else {
break;
@@ -208,17 +196,15 @@
debug("## MATCH ## %s\n", data);
- if (s == NULL) {
- printf("%s=%s\n", name, t);
+ if (!s)
return 1;
- }
old = caps[0].ptr;
olen = caps[0].len;
- if (nlen + 1 >= SLRE_PATSZ) {
+ if (nlen + 1 >= nbuf_size) {
printf("## error: pattern buffer overflow: have %d, need %d\n",
- SLRE_BUFSZ, nlen + 1);
+ nbuf_size, nlen + 1);
return 1;
}
strcpy(nbuf, s);
@@ -263,7 +249,7 @@
break;
np = substitute(np, &nlen,
- SLRE_PATSZ,
+ nbuf_size,
backref, 2,
caps[i].ptr, caps[i].len);
@@ -273,9 +259,8 @@
}
debug("## SUBST(2) ## %s\n", nbuf);
- datap = substitute(datap, &len, SLRE_BUFSZ,
- old, olen,
- nbuf, nlen);
+ datap = substitute(datap, &len, data_size, old, olen,
+ nbuf, nlen);
if (datap == NULL)
return 1;
@@ -289,6 +274,61 @@
}
debug("## FINAL (now env_set()) : %s\n", data);
+ return 0;
+}
+
+#define SLRE_BUFSZ 16384
+#define SLRE_PATSZ 4096
+
+/*
+ * Perform regex operations on a environment variable
+ *
+ * Returns 0 if OK, 1 in case of errors.
+ */
+static int regex_sub_var(const char *name, const char *r, const char *s,
+ const char *t, int global)
+{
+ struct slre slre;
+ char data[SLRE_BUFSZ];
+ char nbuf[SLRE_PATSZ];
+ const char *value;
+ int len;
+ int ret;
+
+ if (!name)
+ return 1;
+
+ if (slre_compile(&slre, r) == 0) {
+ printf("Error compiling regex: %s\n", slre.err_str);
+ return 1;
+ }
+
+ if (!t) {
+ value = env_get(name);
+ if (!value) {
+ printf("## Error: variable \"%s\" not defined\n", name);
+ return 1;
+ }
+ t = value;
+ }
+
+ debug("REGEX on %s=%s\n", name, t);
+ debug("REGEX=\"%s\", SUBST=\"%s\", GLOBAL=%d\n", r, s ? s : "<NULL>",
+ global);
+
+ len = strlen(t);
+ if (len + 1 > SLRE_BUFSZ) {
+ printf("## error: subst buffer overflow: have %d, need %d\n",
+ SLRE_BUFSZ, len + 1);
+ return 1;
+ }
+
+ strcpy(data, t);
+
+ ret = regex_sub(data, SLRE_BUFSZ, nbuf, SLRE_PATSZ, r, s, global);
+ if (ret)
+ return 1;
+
printf("%s=%s\n", name, data);
return env_set(name, data);
@@ -331,10 +371,10 @@
* with 5 args, "t" will be NULL
*/
if (strcmp(argv[2], "gsub") == 0)
- return regex_sub(argv[1], argv[3], argv[4], argv[5], 1);
+ return regex_sub_var(argv[1], argv[3], argv[4], argv[5], 1);
if (strcmp(argv[2], "sub") == 0)
- return regex_sub(argv[1], argv[3], argv[4], argv[5], 0);
+ return regex_sub_var(argv[1], argv[3], argv[4], argv[5], 0);
#endif
/* standard operators: "setexpr name val1 op val2" */