dd: avoid recursive parsing of multipliers

* src/dd.c (parse_integer): Use iterative rather than recursive parsing,
to avoid potential stack overflow.
* tests/dd/bytes.sh: Add a test case.
https://github.com/coreutils/coreutils/issues/254

diff --git a/src/dd.c b/src/dd.c
index 5451b0ac917b8832449a89ea3dbbb6eb7bf8d77a..72dcef1683d6165729be58f9a5102ac8c7d60b72 100644 (file)
--- a/src/dd.c
+++ b/src/dd.c
@@ -1413,48 +1413,72 @@ parse_integer (char const *str, strtol_error *invalid)
      calling code should not rely on this function returning 0
      when *INVALID represents a non-overflow error.  */
   int indeterminate = 0;
-  uintmax_t n = indeterminate;
-  char *suffix;
   static char const suffixes[] = "bcEGkKMPQRTwYZ0";
-  strtol_error e = xstrtoumax (str, &suffix, 10, &n, suffixes);
-  intmax_t result;
+  intmax_t result = 1;
+  bool overflow = false;
+  bool warn_zero_multiplier = false;
+  strtol_error e;
 
-  if ((e & ~LONGINT_OVERFLOW) == LONGINT_INVALID_SUFFIX_CHAR
-      && *suffix == 'B' && str < suffix && suffix[-1] != 'B')
+  while (true)
     {
-      suffix++;
-      if (!*suffix)
-        e &= ~LONGINT_INVALID_SUFFIX_CHAR;
-    }
+      uintmax_t n = indeterminate;
+      char *suffix;
+      e = xstrtoumax (str, &suffix, 10, &n, suffixes);
 
-  if ((e & ~LONGINT_OVERFLOW) == LONGINT_INVALID_SUFFIX_CHAR
-      && *suffix == 'x')
-    {
-      strtol_error f = LONGINT_OK;
-      intmax_t o = parse_integer (suffix + 1, &f);
-      if ((f & ~LONGINT_OVERFLOW) != LONGINT_OK)
+      if ((e & ~LONGINT_OVERFLOW) == LONGINT_INVALID_SUFFIX_CHAR
+          && *suffix == 'B' && str < suffix && suffix[-1] != 'B')
         {
-          e = f;
-          result = indeterminate;
+          suffix++;
+          if (!*suffix)
+            e &= ~LONGINT_INVALID_SUFFIX_CHAR;
         }
-      else if (ckd_mul (&result, n, o)
-               || (result != 0 && ((e | f) & LONGINT_OVERFLOW)))
+
+      bool multiply
+        = ((e & ~LONGINT_OVERFLOW) == LONGINT_INVALID_SUFFIX_CHAR
+           && *suffix == 'x');
+      if (multiply)
+        e &= ~LONGINT_INVALID_SUFFIX_CHAR;
+
+      if ((e & ~LONGINT_OVERFLOW) != LONGINT_OK)
         {
+          if (! (e & LONGINT_OVERFLOW) && n <= INTMAX_MAX)
+            {
+              *invalid = e;
+              return indeterminate;
+            }
           e = LONGINT_OVERFLOW;
-          result = INTMAX_MAX;
         }
-      else
+
+      if (n == 0)
+        result = 0;
+      else if (result != 0)
         {
-          if (result == 0 && STRPREFIX (str, "0x"))
-            diagnose (0, _("warning: %s is a zero multiplier; "
-                           "use %s if that is intended"),
-                      quote_n (0, "0x"), quote_n (1, "00x"));
-          e = LONGINT_OK;
+          intmax_t product;
+          if ((e & LONGINT_OVERFLOW)
+              || overflow
+              || ckd_mul (&product, result, n))
+            overflow = true;
+          else
+            result = product;
         }
+
+      if (multiply && STRPREFIX (str, "0x"))
+        warn_zero_multiplier = true;
+
+      if (! multiply)
+        break;
+      str = suffix + 1;
     }
-  else if (n <= INTMAX_MAX)
-    result = n;
-  else
+
+  if (result == 0)
+    {
+      if (warn_zero_multiplier)
+        diagnose (0, _("warning: %s is a zero multiplier; "
+                       "use %s if that is intended"),
+                  quote_n (0, "0x"), quote_n (1, "00x"));
+      e = LONGINT_OK;
+    }
+  else if (overflow)
     {
       e = LONGINT_OVERFLOW;
       result = INTMAX_MAX;

diff --git a/tests/dd/bytes.sh b/tests/dd/bytes.sh
index 73ad2eb5012d309e6ba92e4e077d649f4717207b..fa7295ac8ae6c5e27a3d294610e330efd6a3cf6b 100755 (executable)
--- a/tests/dd/bytes.sh
+++ b/tests/dd/bytes.sh
@@ -60,7 +60,7 @@ for operands in "oseek=8B" "seek=8 oflag=seek_bytes"; do
   compare expected2 out2 || fail=1
 done
 
-# Check recursive integer parsing
+# Check multiplicative integer parsing
 for oseek in '1x2x4 oflag=seek_bytes' '1Bx2x4' '1Bx8' '2Bx4B' '2x4B'; do
   # seek bytes
   echo abcdefghijklm |
@@ -68,6 +68,13 @@ for oseek in '1x2x4 oflag=seek_bytes' '1Bx2x4' '1Bx8' '2Bx4B' '2x4B'; do
   compare expected out || fail=1
 done
 
+# Check that long multiplier chains don't exhaust a restricted stack.
+if (ulimit -S -s 256 && dd if=/dev/null count=1) 2>/dev/null; then
+  long_multiplier=$(yes 1x | head -n 10000 | tr -d '\n')1 || framework_failure_
+  (ulimit -S -s 256 &&
+    dd count="$long_multiplier" if=/dev/null of=/dev/null status=none) || fail=1
+fi
+
 # Negative checks for integer parsing
 for count in B B1 Bx1 KBB BB KBb KBx x1 1x 1xx1; do
   returns_ 1 dd count=$count </dev/null >/dev/null || fail=1