numfmt: fix buffer over-read (CWE-126)

* src/numfmt.c (simple_strtod_human): Check for NULL after pointer
adjustment to avoid Out-of-range pointer offset (CWE-823).
* NEWS: Mention the fix.

diff --git a/NEWS b/NEWS
index aa7c13f97c5ccb8a96f5c199e972ed5e08423588..e6053a04b3d78746eddccd64f11fdc2b09f10a23 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   Also non standard SHA2 tags with a bad length resulted in undefined behavior.
   [bug introduced in coreutils-9.8]
 
+  'numfmt' no longer reads out-of-bounds memory with trailing blanks in input.
+  [bug introduced with numfmt in coreutils-8.21]
+
   'rm -d DIR' no longer fails on Ceph snapshot directories.
   Although these directories are nonempty, 'rmdir DIR' succeeds on them.
   [bug introduced in coreutils-8.16]

diff --git a/src/numfmt.c b/src/numfmt.c
index 4f72facb94447f0e24a4abbd9826d4a6be012c86..0cc12689e2c64aa366035d46a748bef0597c256c 100644 (file)
--- a/src/numfmt.c
+++ b/src/numfmt.c
@@ -641,7 +641,7 @@ simple_strtod_human (char const *input_str,
   devmsg ("  parsed numeric value: %Lf\n"
           "  input precision = %d\n", *value, (int)*precision);
 
-  if (**endptr != '\0')
+  while (**endptr)
     {
       /* process suffix.  */
 
@@ -649,6 +649,9 @@ simple_strtod_human (char const *input_str,
       while (isblank (to_uchar (**endptr)))
         (*endptr)++;
 
+      if (**endptr == '\0')
+        break;  /* Treat as no suffix.  */
+
       if (!valid_suffix (**endptr))
         return SSE_INVALID_SUFFIX;
 
@@ -661,9 +664,9 @@ simple_strtod_human (char const *input_str,
       if (allowed_scaling == scale_auto && **endptr == 'i')
         {
           /* auto-scaling enabled, and the first suffix character
-              is followed by an 'i' (e.g. Ki, Mi, Gi).  */
+             is followed by an 'i' (e.g. Ki, Mi, Gi).  */
           scale_base = 1024;
-          (*endptr)++;              /* skip second  ('i') suffix character.  */
+          (*endptr)++;              /* skip 'i' in suffix.  */
           devmsg ("  Auto-scaling, found 'i', switching to base %d\n",
                   scale_base);
         }
@@ -676,6 +679,8 @@ simple_strtod_human (char const *input_str,
         }
 
       *precision = 0;  /* Reset, to select precision based on scale.  */
+
+      break;
     }
 
   long double multiplier = powerld (scale_base, power);

diff --git a/tests/misc/numfmt.pl b/tests/misc/numfmt.pl
index 051db785c2ba846f49e0c3508b41653b4d539268..4dd9718c9cbf1ab52390556b4eb70d83319ec46e 100755 (executable)
--- a/tests/misc/numfmt.pl
+++ b/tests/misc/numfmt.pl
@@ -163,6 +163,7 @@ my @Tests =
      ['suf-20',
       '--suffix=Foo' . 'x' x 122 . 'y 0',
       {OUT => '0Foo' . 'x' x 122 . 'y'}],
+     ['suf-21', "-d '' --from=si '4  '",         {OUT => "4"}],
 
      ## GROUPING