From e7f946e5b88455b76c21625dc1e3f73f4f9483c5 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 23 Nov 2016 00:05:51 -0800 Subject: [PATCH] pr: fix integer overflow in buffer size calcs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- configure.ac | 2 ++ src/pr.c | 59 +++++++++++++++++++++++++++++++++++++---------------------- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/configure.ac b/configure.ac index a484601..1e74b36 100644 --- 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 4e9d12e..20e8637 100644 --- 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; -- 2.7.4