xsparse cleanup, including integer overflow

* scripts/xsparse.c: Include inttypes.h, for strtoimax.
Don’t include stdint.h, since inttypes.h includes it.
Sort include directives.
Make all extern functions and vars static, except for ‘main’.
(string_to_off): Use strtoimax instead of doing overflow
checking by hand, incorrectly (it relied on undefined behavior).
(string_to_size): New arg MAXSIZE.  All callers changed.
(get_var): Return bool not int.  Fix unlikely integer overflow.
Use strncmp instead of memcmp, to avoid unlikely pointer overflow.
(read_xheader, read_map, main): Avoid unlikely integer overflow.
Check for I/O errors more consistently.
(main): Prefer bool to int, and put vars near use.

diff --git a/scripts/xsparse.c b/scripts/xsparse.c
index 7aca888ce58f4ff1e671579787a9125f85ed052c..5d7166ae642cc49b43989dd721810e3a67a39b71 100644 (file)
--- a/scripts/xsparse.c
+++ b/scripts/xsparse.c
@@ -20,16 +20,17 @@
 
    Written by Sergey Poznyakoff  */
 
-#include <stdlib.h>
-#include <stdint.h>
-#include <stdio.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <limits.h>
 #include <stdarg.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
-#include <unistd.h>
-#include <fcntl.h>
 #include <sys/stat.h>
-#include <limits.h>
-#include <errno.h>
+#include <unistd.h>
 
 /* Bound on length of the string representing an off_t.
    See INT_STRLEN_BOUND in intprops.h for explanation */
@@ -44,10 +45,10 @@ struct sp_array
   off_t numbytes;
 };
 
-char *progname;
-int verbose;
+static char *progname;
+static bool verbose;
 
-void
+static void
 die (int code, char *fmt, ...)
 {
   va_list ap;
@@ -60,7 +61,7 @@ die (int code, char *fmt, ...)
   exit (code);
 }
 
-void *
+static void *
 emalloc (size_t size)
 {
   char *p = malloc (size);
@@ -69,49 +70,33 @@ emalloc (size_t size)
   return p;
 }
 
-off_t
+static off_t
 string_to_off (char *p, char **endp)
 {
-  off_t v = 0;
-
-  for (; *p; p++)
-    {
-      int digit = *p - '0';
-      off_t x = v * 10;
-      if (9 < (unsigned) digit)
-       {
-         if (endp)
-           {
-             *endp = p;
-             break;
-           }
-         die (1, "number parse error near %s", p);
-       }
-      else if (x / 10 != v)
-       die (1, "number out of allowed range, near %s", p);
-      v = x + digit;
-      if (v < 0)
-       die (1, "negative number");
-    }
-  if (endp)
-    *endp = p;
+  errno = 0;
+  intmax_t i = strtoimax (p, endp, 10);
+  off_t v = i;
+  if (i < 0 || v != i || errno == ERANGE)
+    die (1, "number out of allowed range, near %s", p);
+  if (errno || p == *endp)
+    die (1, "number parse error near %s", p);
   return v;
 }
 
-size_t
-string_to_size (char *p, char **endp)
+static size_t
+string_to_size (char *p, char **endp, size_t maxsize)
 {
   off_t v = string_to_off (p, endp);
   size_t ret = v;
-  if (ret != v)
+  if (! (ret == v && ret <= maxsize))
     die (1, "number too big");
   return ret;
 }
 
-size_t sparse_map_size;
-struct sp_array *sparse_map;
+static size_t sparse_map_size;
+static struct sp_array *sparse_map;
 
-void
+static void
 get_line (char *s, int size, FILE *stream)
 {
   char *p = fgets (s, size, stream);
@@ -122,10 +107,10 @@ get_line (char *s, int size, FILE *stream)
   len = strlen (p);
   if (s[len - 1] != '\n')
     die (1, "buffer overflow");
-  s[len - 1] = 0;
+  s[len - 1] = '\0';
 }
 
-int
+static bool
 get_var (FILE *fp, char **name, char **value)
 {
   static char *buffer;
@@ -138,12 +123,12 @@ get_var (FILE *fp, char **name, char **value)
       size_t len, s;
 
       if (!fgets (buffer, bufsize, fp))
-       return 0;
+       return false;
       len = strlen (buffer);
       if (len == 0)
-       return 0;
+       return false;
 
-      s = string_to_size (buffer, &p);
+      s = string_to_size (buffer, &p, SIZE_MAX - 1);
       if (*p != ' ')
        die (1, "malformed header: expected space but found %s", p);
       if (buffer[len-1] != '\n')
@@ -160,7 +145,7 @@ get_var (FILE *fp, char **name, char **value)
        }
       p++;
     }
-  while (memcmp (p, "GNU.sparse.", 11));
+  while (strncmp (p, "GNU.sparse.", 11) != 0);
 
   p += 11;
   q = strchr (p, '=');
@@ -170,15 +155,15 @@ get_var (FILE *fp, char **name, char **value)
   q[strlen (q) - 1] = 0;
   *name = p;
   *value = q;
-  return 1;
+  return true;
 }
 
-char *outname;
-off_t outsize;
-unsigned version_major;
-unsigned version_minor;
+static char *outname;
+static off_t outsize;
+static unsigned int version_major;
+static unsigned int version_minor;
 
-void
+static void
 read_xheader (char *name)
 {
   char *kw, *val;
@@ -205,11 +190,11 @@ read_xheader (char *name)
        }
       else if (strcmp (kw, "major") == 0)
        {
-         version_major = string_to_size (val, NULL);
+         version_major = string_to_size (val, NULL, SIZE_MAX);
        }
       else if (strcmp (kw, "minor") == 0)
        {
-         version_minor = string_to_size (val, NULL);
+         version_minor = string_to_size (val, NULL, SIZE_MAX);
        }
       else if (strcmp (kw, "realsize") == 0
               || strcmp (kw, "size") == 0)
@@ -218,7 +203,8 @@ read_xheader (char *name)
        }
       else if (strcmp (kw, "numblocks") == 0)
        {
-         sparse_map_size = string_to_size (val, NULL);
+         sparse_map_size = string_to_size (val, NULL,
+                                           SIZE_MAX / sizeof *sparse_map);
          sparse_map = emalloc (sparse_map_size * sizeof *sparse_map);
        }
       else if (strcmp (kw, "offset") == 0)
@@ -258,10 +244,11 @@ read_xheader (char *name)
     die (1, "size of the sparse map unknown");
   if (i != sparse_map_size)
     die (1, "not all sparse entries supplied");
-  fclose (fp);
+  if (ferror (fp) || fclose (fp) < 0)
+    die (1, "read error: %s", name);
 }
 
-void
+static void
 read_map (FILE *ifp)
 {
   size_t i;
@@ -271,7 +258,7 @@ read_map (FILE *ifp)
     printf ("Reading v.1.0 sparse map\n");
 
   get_line (nbuf, sizeof nbuf, ifp);
-  sparse_map_size = string_to_size (nbuf, NULL);
+  sparse_map_size = string_to_size (nbuf, NULL, SIZE_MAX / sizeof *sparse_map);
   sparse_map = emalloc (sparse_map_size * sizeof *sparse_map);
 
   for (i = 0; i < sparse_map_size; i++)
@@ -282,11 +269,15 @@ read_map (FILE *ifp)
       sparse_map[i].numbytes = string_to_off (nbuf, NULL);
     }
 
-  fseeko (ifp, ((ftell (ifp) + BLOCKSIZE - 1) / BLOCKSIZE) * BLOCKSIZE,
-         SEEK_SET);
+  off_t ifp_offset = ftello (ifp);
+  if (ifp_offset < 0)
+    die (1, "ftello");
+  if (ifp_offset % BLOCKSIZE != 0
+      && fseeko (ifp, BLOCKSIZE - ifp_offset % BLOCKSIZE, SEEK_CUR) < 0)
+    die (1, "fseeko");
 }
 
-void
+static void
 expand_sparse (FILE *sfp, int ofd)
 {
   size_t i;
@@ -327,7 +318,7 @@ expand_sparse (FILE *sfp, int ofd)
   free (buffer);
 }
 
-void
+static void
 usage (int code)
 {
   printf ("Usage: %s [OPTIONS] infile [outfile]\n", progname);
@@ -342,7 +333,7 @@ usage (int code)
   exit (code);
 }
 
-void
+static void
 guess_outname (char *name)
 {
   char *p;
@@ -388,12 +379,8 @@ int
 main (int argc, char **argv)
 {
   int c;
-  int dry_run = 0;
+  bool dry_run = false;
   char *xheader_file = NULL;
-  char *inname;
-  FILE *ifp;
-  struct stat st;
-  int ofd;
 
   progname = argv[0];
   while ((c = getopt (argc, argv, "hnvx:")) != EOF)
@@ -409,9 +396,9 @@ main (int argc, char **argv)
          break;
 
        case 'n':
-         dry_run = 1;
+         dry_run = true;
        case 'v':
-         verbose++;
+         verbose = true;
          break;
 
        default:
@@ -428,15 +415,16 @@ main (int argc, char **argv)
   if (xheader_file)
     read_xheader (xheader_file);
 
-  inname = argv[0];
+  char *inname = argv[0];
   if (argv[1])
     outname = argv[1];
 
-  if (stat (inname, &st))
+  struct stat st;
+  if (stat (inname, &st) < 0)
     die (1, "cannot stat %s (%d)", inname, errno);
 
-  ifp = fopen (inname, "r");
-  if (ifp == NULL)
+  FILE *ifp = fopen (inname, "r");
+  if (!ifp)
     die (1, "cannot open file %s (%d)", inname, errno);
 
   if (!xheader_file || version_major == 1)
@@ -445,8 +433,8 @@ main (int argc, char **argv)
   if (!outname)
     guess_outname (inname);
 
-  ofd = open (outname, O_RDWR|O_CREAT|O_TRUNC, st.st_mode);
-  if (ofd == -1)
+  int ofd = open (outname, O_RDWR|O_CREAT|O_TRUNC, st.st_mode);
+  if (ofd < 0)
     die (1, "cannot open file %s (%d)", outname, errno);
 
   if (verbose)
@@ -460,15 +448,17 @@ main (int argc, char **argv)
 
   expand_sparse (ifp, ofd);
 
-  fclose (ifp);
-  close (ofd);
+  if (ferror (ifp) || fclose (ifp) < 0)
+    die (1, "input error: %s", inname);
+  if (close (ofd) < 0)
+    die (1, "output error: %s", outname);
 
   if (verbose)
     printf ("Done\n");
 
   if (outsize)
     {
-      if (stat (outname, &st))
+      if (stat (outname, &st) < 0)
        die (1, "cannot stat output file %s (%d)", outname, errno);
       if (st.st_size != outsize)
        die (1, "expanded file has wrong size");