pr: fix integer overflow in buffer size calcs

Problem reported by Marcel Böhme (Bug#24996).
* configure.ac (WERROR_CFLAGS): Avoid -Wtype-limits.
* src/pr.c (col_sep_string): Now a const pointer.  All uses changed.
(integer_overflow): New function.
(separator_string, main, init_parameters, init_store_cols):
Check for integer overflow.
(align_column, read_line, print_stored): Avoid integer overflow.

diff --git a/configure.ac b/configure.ac
index a48460114a873281332cb063db8282fbbcd5151a..1e74b361facc9bbdada609ad7c376c943dfe308f 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -132,6 +132,7 @@ if test "$gl_gcc_warnings" = yes; then
   nw="$nw -Wswitch-enum"            # Too many warnings for now
   nw="$nw -Wswitch-default"         # Too many warnings for now
   nw="$nw -Wstack-protector"        # not worth working around
+  nw="$nw -Wtype-limits"            # False alarms for portable code
   # things I might fix soon:
   nw="$nw -Wfloat-equal"            # sort.c, seq.c
   nw="$nw -Wmissing-format-attribute" # copy.c
@@ -150,6 +151,7 @@ if test "$gl_gcc_warnings" = yes; then
     gl_WARN_ADD([$w])
   done
   gl_WARN_ADD([-Wno-sign-compare])     # Too many warnings for now
+  gl_WARN_ADD([-Wno-type-limits])      # False alarms for portable code
   gl_WARN_ADD([-Wno-unused-parameter]) # Too many warnings for now
   gl_WARN_ADD([-Wno-format-nonliteral])
 

diff --git a/src/pr.c b/src/pr.c
index 4e9d12e3c3a3f1a40201157f17925a70b2aa139c..20e8637606b986419dfd0e87d4fe7eb5ff16a42b 100644 (file)
--- a/src/pr.c
+++ b/src/pr.c
@@ -688,7 +688,7 @@ static bool use_col_separator = false;
 /* String used to separate columns if the -S option has been specified.
    Default without -S but together with one of the column options
    -a|COLUMN|-m is a 'space' and with the -J option a 'tab'. */
-static char *col_sep_string = (char *) "";
+static char const *col_sep_string = "";
 static int col_sep_length = 0;
 static char *column_separator = (char *) " ";
 static char *line_separator = (char *) "\t";
@@ -771,6 +771,12 @@ static struct option const long_options[] =
   {NULL, 0, NULL, 0}
 };
 
+static void
+integer_overflow (void)
+{
+  die (EXIT_FAILURE, 0, _("integer overflow"));
+}
+
 /* Return the number of columns that have either an open file or
    stored lines. */
 
@@ -840,9 +846,11 @@ parse_column_count (char const *s)
 static void
 separator_string (const char *optarg_S)
 {
-  col_sep_length = (int) strlen (optarg_S);
-  col_sep_string = xmalloc (col_sep_length + 1);
-  strcpy (col_sep_string, optarg_S);
+  size_t len = strlen (optarg_S);
+  if (INT_MAX < len)
+    integer_overflow ();
+  col_sep_length = len;
+  col_sep_string = optarg_S;
 }
 
 int
@@ -869,7 +877,7 @@ main (int argc, char **argv)
 
   n_files = 0;
   file_names = (argc > 1
-                ? xmalloc ((argc - 1) * sizeof (char *))
+                ? xnmalloc (argc - 1, sizeof (char *))
                 : NULL);
 
   while (true)
@@ -1000,7 +1008,7 @@ main (int argc, char **argv)
         case 'S':
           old_s = false;
           /* Reset an additional input of -s, -S dominates -s */
-          col_sep_string = bad_cast ("");
+          col_sep_string = "";
           col_sep_length = 0;
           use_col_separator = true;
           if (optarg)
@@ -1262,8 +1270,13 @@ init_parameters (int number_of_files)
         chars_used_by_number = number_width;
     }
 
-  chars_per_column = (chars_per_line - chars_used_by_number
-                      - (columns - 1) * col_sep_length) / columns;
+  int sep_chars, useful_chars;
+  if (INT_MULTIPLY_WRAPV (columns - 1, col_sep_length, &sep_chars))
+    sep_chars = INT_MAX;
+  if (INT_SUBTRACT_WRAPV (chars_per_line - chars_used_by_number, sep_chars,
+                          &useful_chars))
+    useful_chars = 0;
+  chars_per_column = useful_chars / columns;
 
   if (chars_per_column < 1)
     die (EXIT_FAILURE, 0, _("page width too narrow"));
@@ -1714,7 +1727,7 @@ static void
 align_column (COLUMN *p)
 {
   padding_not_printed = p->start_position;
-  if (padding_not_printed - col_sep_length > 0)
+  if (col_sep_length < padding_not_printed)
     {
       pad_across_to (padding_not_printed - col_sep_length);
       padding_not_printed = ANYWHERE;
@@ -1877,21 +1890,25 @@ print_page (void)
 static void
 init_store_cols (void)
 {
-  int total_lines = lines_per_body * columns;
-  int chars_if_truncate = total_lines * (chars_per_column + 1);
+  int total_lines, total_lines_1, chars_per_column_1, chars_if_truncate;
+  if (INT_MULTIPLY_WRAPV (lines_per_body, columns, &total_lines)
+      || INT_ADD_WRAPV (total_lines, 1, &total_lines_1)
+      || INT_ADD_WRAPV (chars_per_column, 1, &chars_per_column_1)
+      || INT_MULTIPLY_WRAPV (total_lines, chars_per_column_1,
+                             &chars_if_truncate))
+    integer_overflow ();
 
   free (line_vector);
   /* FIXME: here's where it was allocated.  */
-  line_vector = xmalloc ((total_lines + 1) * sizeof *line_vector);
+  line_vector = xnmalloc (total_lines_1, sizeof *line_vector);
 
   free (end_vector);
-  end_vector = xmalloc (total_lines * sizeof *end_vector);
+  end_vector = xnmalloc (total_lines, sizeof *end_vector);
 
   free (buff);
-  buff_allocated = (use_col_separator
-                    ? 2 * chars_if_truncate
-                    : chars_if_truncate);      /* Tune this. */
-  buff = xmalloc (buff_allocated);
+  buff = xnmalloc (chars_if_truncate, use_col_separator + 1);
+  buff_allocated = chars_if_truncate;  /* Tune this. */
+  buff_allocated *= use_col_separator + 1;
 }
 
 /* Store all but the rightmost column.
@@ -2204,11 +2221,9 @@ print_white_space (void)
 static void
 print_sep_string (void)
 {
-  char *s;
+  char const *s = col_sep_string;
   int l = col_sep_length;
 
-  s = col_sep_string;
-
   if (separators_not_printed <= 0)
     {
       /* We'll be starting a line with chars_per_margin, anything else? */
@@ -2468,7 +2483,7 @@ read_line (COLUMN *p)
           align_empty_cols = false;
         }
 
-      if (padding_not_printed - col_sep_length > 0)
+      if (col_sep_length < padding_not_printed)
         {
           pad_across_to (padding_not_printed - col_sep_length);
           padding_not_printed = ANYWHERE;
@@ -2571,7 +2586,7 @@ print_stored (COLUMN *p)
         }
     }
 
-  if (padding_not_printed - col_sep_length > 0)
+  if (col_sep_length < padding_not_printed)
     {
       pad_across_to (padding_not_printed - col_sep_length);
       padding_not_printed = ANYWHERE;