Re: Expr: Improvement of substr

[Paul Eggert]

Thanks for reporting that.  The problem turns out to be worse than
what you state, since "expr substr hello 1 4294967296" has a buffer
overrun on my 32-bit host (unfortunately your patch doesn't fix this).

Writing a portable test case for this is a bit tricky since we
currently don't assume int types wider than 32 bits, but anyway I
fixed both the performance bug and the buffer overrun as follows.

2006-09-10  Paul Eggert  <address@hidden>

        * src/expr.c (eval6): Fix buffer overrun, or bad performance, if
        substr's last operand is very large.  Performance problem reported
        by Sebastian Kreft.

--- src/expr.c  8 Jun 2006 02:53:25 -0000       1.114
+++ src/expr.c  11 Sep 2006 04:56:43 -0000      1.115
@@ -551,21 +551,25 @@ eval6 (bool evaluate)
     }
   else if (nextarg ("substr"))
     {
+      size_t llen;
       l = eval6 (evaluate);
       i1 = eval6 (evaluate);
       i2 = eval6 (evaluate);
       tostring (l);
+      llen = strlen (l->u.s);
       if (!toarith (i1) || !toarith (i2)
-         || strlen (l->u.s) < i1->u.i
+         || llen < i1->u.i
          || i1->u.i <= 0 || i2->u.i <= 0)
        v = str_value ("");
       else
        {
+         size_t vlen = MIN (i2->u.i, llen - i1->u.i + 1);
+         char *vlim;
          v = xmalloc (sizeof *v);
          v->type = string;
-         v->u.s = strncpy (xmalloc (i2->u.i + 1),
-                           l->u.s + i1->u.i - 1, i2->u.i);
-         v->u.s[i2->u.i] = 0;
+         v->u.s = xmalloc (vlen + 1);
+         vlim = mempcpy (v->u.s, l->u.s + i1->u.i - 1, vlen);
+         *vlim = '\0';
        }
       freev (l);
       freev (i1);