From a891a95eff7546663572e71ee6c9263e9d091dac Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 12 Mar 2024 12:27:48 -0700 Subject: [PATCH 5/5] printf now diagnoses more width/prec overflow Diagnose overflow when printf parses width or precision itself as opposed to letting the underlying printf do it. * printf.def: Include stdckdint.h. (decodeint): New function, replacing decodeprec; name changed since it also scans widths. All uses changed. (printf_builtin, printstr, printwidestr): Diagnose out-of-range widths and precisions instead of relying on undefined behavior, or silently doing the wrong thing. (getint): New arg IFOVERFLOW. All uses changed. Check for overflow properly. --- builtins/printf.def | 149 ++++++++++++++++++++------------------------ 1 file changed, 66 insertions(+), 83 deletions(-) diff --git a/builtins/printf.def b/builtins/printf.def index 27e00a9e..5b6a3718 100644 --- a/builtins/printf.def +++ b/builtins/printf.def @@ -69,7 +69,7 @@ $END #endif #include - +#include #include #include @@ -191,7 +191,7 @@ extern int vsnprintf (char *, size_t, const char *, va_list) __attribute__((__fo #endif static void printf_erange (char *); -static int printstr (char *, char *, int, int, int); +static int printstr (char *, char *, int, int, int, int); static int tescape (char *, char *, int *, int *); static char *bexpand (char *, int, int *, int *); static char *vbadd (char *, int); @@ -199,7 +199,7 @@ static int vbprintf (const char *, ...) __attribute__((__format__ (printf, 1, 2) static char *mklong (char *, char *, size_t); static int getchr (void); static char *getstr (void); -static int getint (void); +static int getint (int); static intmax_t getintmax (void); static uintmax_t getuintmax (void); @@ -224,7 +224,7 @@ static wchar_t *getwidestr (size_t *); static wint_t getwidechar (void); static char *convwidestr (wchar_t *, int); static char *convwidechar (wint_t, int); -static int printwidestr (char *, wchar_t *, size_t, int, int); +static int printwidestr (char *, wchar_t *, size_t, int, int, int); #endif static WORD_LIST *garglist, *orig_arglist; @@ -243,22 +243,38 @@ static intmax_t tw; static char *conv_buf; static size_t conv_bufsize; +/* Convert the unsigned decimal integer at the start of *STR to int. + **STR must be a digit. + Update *STR to point past the end of the integer. + If DIAGNOSE, diagnose if the integer does not fit in int. + Return the scanned value, or IFOVERFLOW if it does fit in int. */ static inline int -decodeprec (char *ps) +decodeint (char **str, int diagnose, int ifoverflow) { - intmax_t mpr; + char *ps = *str; + int pr = *ps++ - '0'; + int v = 0; - mpr = *ps++ - '0'; - while (DIGIT (*ps)) - mpr = (mpr * 10) + (*ps++ - '0'); - return (mpr < 0 || mpr > INT_MAX) ? INT_MAX : mpr; + for (; DIGIT (*ps); ps++) + { + v |= ckd_mul (&pr, pr, 10); + v |= ckd_add (&pr, pr, *ps - '0'); + } + if (v && diagnose) + { + char *prefix = substring (*str, 0, ps - *str); + printf_erange (prefix); + free (prefix); + } + *str = ps; + return v ? ifoverflow : pr; } int printf_builtin (WORD_LIST *list) { int ch, fieldwidth, precision; - int have_fieldwidth, have_precision, use_Lmod, altform, longform; + int have_fieldwidth, have_precision, use_Lmod, altform, longform, diagnose; char convch, thisch, nextch, *format, *modstart, *precstart, *fmt, *start; #if defined (HANDLE_MULTIBYTE) char mbch[25]; /* 25 > MB_LEN_MAX, plus can handle 4-byte UTF-8 and large Unicode characters*/ @@ -329,6 +345,7 @@ printf_builtin (WORD_LIST *list) format = list->word->word; tw = 0; + diagnose = 1; retval = EXECUTION_SUCCESS; garglist = orig_arglist = list->next; @@ -406,10 +423,7 @@ printf_builtin (WORD_LIST *list) { fmt++; have_fieldwidth = 1; - fieldwidth = getint (); - /* Handle field with overflow by ignoring it. */ - if (fieldwidth == INT_MAX || fieldwidth == INT_MIN) - fieldwidth = 0; + fieldwidth = getint (0); } else while (DIGIT (*fmt)) @@ -423,11 +437,7 @@ printf_builtin (WORD_LIST *list) { fmt++; have_precision = 1; - precision = getint (); - /* Handle precision overflow by ignoring it. "A negative - precision is treated as if it were missing." */ - if (precision == INT_MAX || precision == INT_MIN) - precision = -1; + precision = getint (-1); } else { @@ -491,9 +501,11 @@ printf_builtin (WORD_LIST *list) /* If %lc is supplied a null argument, posix interp 1647 says it should produce a single null byte. */ if (wc == L'\0') - r = printstr (start, "", 1, fieldwidth, precision); + r = printstr (start, "", 1, fieldwidth, precision, + diagnose); else - r = printwidestr (start, ws, 1, fieldwidth, precision); + r = printwidestr (start, ws, 1, fieldwidth, precision, + diagnose); if (r < 0) PRETURN (EXECUTION_FAILURE); break; @@ -516,7 +528,8 @@ printf_builtin (WORD_LIST *list) int r; wp = getwidestr (&slen); - r = printwidestr (start, wp, slen, fieldwidth, precision); + r = printwidestr (start, wp, slen, fieldwidth, precision, + diagnose); FREE (wp); if (r < 0) PRETURN (EXECUTION_FAILURE); @@ -547,7 +560,8 @@ printf_builtin (WORD_LIST *list) { /* Have to use printstr because of possible NUL bytes in XP -- printf does not handle that well. */ - r = printstr (start, xp, rlen, fieldwidth, precision); + r = printstr (start, xp, rlen, fieldwidth, precision, + diagnose); if (r < 0) retval = EXECUTION_FAILURE; free (xp); @@ -622,7 +636,9 @@ printf_builtin (WORD_LIST *list) /* convert to %s format that preserves fieldwidth and precision */ modstart[0] = 's'; modstart[1] = '\0'; - r = printstr (start, timebuf, strlen (timebuf), fieldwidth, precision); /* XXX - %s for now */ + /* XXX - %s for now */ + r = printstr (start, timebuf, strlen (timebuf), + fieldwidth, precision, diagnose); if (r < 0) PRETURN (EXECUTION_FAILURE); break; @@ -659,7 +675,10 @@ printf_builtin (WORD_LIST *list) if (convch == 'Q' && (have_precision || precstart)) { if (precstart) - precision = decodeprec (precstart); + { + char *prec = precstart; + precision = decodeint (&prec, 0, -1); + } slen = strlen (p); /* printf precision works in bytes. */ if (precision >= 0 && precision < slen) @@ -682,7 +701,8 @@ printf_builtin (WORD_LIST *list) precision = slen; } /* Use printstr to get fieldwidth and precision right. */ - r = printstr (start, xp, slen, fieldwidth, precision); + r = printstr (start, xp, slen, fieldwidth, precision, + diagnose); /* Let PRETURN print the error message. */ free (xp); } @@ -699,8 +719,8 @@ printf_builtin (WORD_LIST *list) long p; intmax_t pp; - p = pp = getintmax (); - if (p != pp) + pp = getintmax (); + if (! (LONG_MIN <= pp && pp <= LONG_MAX)) { f = mklong (start, PRIdMAX, sizeof (PRIdMAX) - 2); PF (f, pp); @@ -711,6 +731,7 @@ printf_builtin (WORD_LIST *list) in "long". This also works around some long long and/or intmax_t library bugs in the common case, e.g. glibc 2.2 x86. */ + p = pp; f = mklong (start, "l", 1); PF (f, p); } @@ -789,6 +810,8 @@ printf_builtin (WORD_LIST *list) /* PRETURN will print error message. */ PRETURN (EXECUTION_FAILURE); } + + diagnose = 0; } while (garglist && garglist != list->next); @@ -815,14 +838,14 @@ printf_erange (char *s) Returns -1 on detectable write error, 0 otherwise. */ static int -printstr (char *fmt, char *string, int len, int fieldwidth, int precision) +printstr (char *fmt, char *string, int len, int fieldwidth, int precision, + int diagnose) { #if 0 char *s; #endif int padlen, nc, ljust, i; int fw, pr; /* fieldwidth and precision */ - intmax_t mfw, mpr; if (string == 0) string = ""; @@ -833,10 +856,8 @@ printstr (char *fmt, char *string, int len, int fieldwidth, int precision) if (*fmt == '%') fmt++; - ljust = fw = 0; + ljust = 0; pr = -1; - mfw = 0; - mpr = -1; /* skip flags */ while (strchr (SKIP1, *fmt)) @@ -858,13 +879,7 @@ printstr (char *fmt, char *string, int len, int fieldwidth, int precision) } } else if (DIGIT (*fmt)) - { - mfw = *fmt++ - '0'; - while (DIGIT (*fmt)) - mfw = (mfw * 10) + (*fmt++ - '0'); - /* Error if fieldwidth > INT_MAX here? */ - fw = (mfw < 0 || mfw > INT_MAX) ? INT_MAX : mfw; - } + fw = decodeint (&fmt, diagnose, 0); /* get precision, if present. doesn't handle negative precisions */ if (*fmt == '.') @@ -876,15 +891,7 @@ printstr (char *fmt, char *string, int len, int fieldwidth, int precision) pr = precision; } else if (DIGIT (*fmt)) - { - mpr = *fmt++ - '0'; - while (DIGIT (*fmt)) - mpr = (mpr * 10) + (*fmt++ - '0'); - /* Error if precision > INT_MAX here? */ - pr = (mpr < 0 || mpr > INT_MAX) ? INT_MAX : mpr; - if (pr < precision && precision < INT_MAX) - pr = precision; /* XXX */ - } + pr = decodeint (&fmt, diagnose, -1); else pr = 0; /* "a null digit string is treated as zero" */ } @@ -925,7 +932,8 @@ printstr (char *fmt, char *string, int len, int fieldwidth, int precision) #if defined (HANDLE_MULTIBYTE) /* A wide-character version of printstr */ static int -printwidestr (char *fmt, wchar_t *wstring, size_t len, int fieldwidth, int precision) +printwidestr (char *fmt, wchar_t *wstring, size_t len, + int fieldwidth, int precision, int diagnose) { #if 0 char *s; @@ -933,7 +941,6 @@ printwidestr (char *fmt, wchar_t *wstring, size_t len, int fieldwidth, int preci char *string; int padlen, nc, ljust, i; int fw, pr; /* fieldwidth and precision */ - intmax_t mfw, mpr; if (wstring == 0) wstring = L""; @@ -946,8 +953,6 @@ printwidestr (char *fmt, wchar_t *wstring, size_t len, int fieldwidth, int preci ljust = fw = 0; pr = -1; - mfw = 0; - mpr = -1; /* skip flags */ while (strchr (SKIP1, *fmt)) @@ -969,13 +974,7 @@ printwidestr (char *fmt, wchar_t *wstring, size_t len, int fieldwidth, int preci } } else if (DIGIT (*fmt)) - { - mfw = *fmt++ - '0'; - while (DIGIT (*fmt)) - mfw = (mfw * 10) + (*fmt++ - '0'); - /* Error if fieldwidth > INT_MAX here? */ - fw = (mfw < 0 || mfw > INT_MAX) ? INT_MAX : mfw; - } + fw = decodeint (&fmt, diagnose, 0); /* get precision, if present. doesn't handle negative precisions */ if (*fmt == '.') @@ -987,15 +986,7 @@ printwidestr (char *fmt, wchar_t *wstring, size_t len, int fieldwidth, int preci pr = precision; } else if (DIGIT (*fmt)) - { - mpr = *fmt++ - '0'; - while (DIGIT (*fmt)) - mpr = (mpr * 10) + (*fmt++ - '0'); - /* Error if precision > INT_MAX here? */ - pr = (mpr < 0 || mpr > INT_MAX) ? INT_MAX : mpr; - if (pr < precision && precision < INT_MAX) - pr = precision; /* XXX */ - } + pr = decodeint (&fmt, diagnose, -1); else pr = 0; /* "a null digit string is treated as zero" */ } @@ -1345,10 +1336,11 @@ getstr (void) /* Don't call getintmax here because it may consume an argument on error, and we call this to get field width/precision arguments. */ static int -getint (void) +getint (int ifoverflow) { intmax_t ret; char *ep; + int overflow; if (garglist == 0) return (0); @@ -1358,27 +1350,18 @@ getint (void) errno = 0; ret = strtoimax (garglist->word->word, &ep, 0); + overflow = errno == ERANGE || ! (-INT_MAX <= ret && ret <= INT_MAX); if (*ep) { sh_invalidnum (garglist->word->word); conversion_error = 1; } - else if (errno == ERANGE) + else if (overflow) printf_erange (garglist->word->word); - else if (ret > INT_MAX) - { - printf_erange (garglist->word->word); - ret = INT_MAX; - } - else if (ret < INT_MIN) - { - printf_erange (garglist->word->word); - ret = INT_MIN; - } garglist = garglist->next; - return ((int)ret); + return (overflow ? ifoverflow : (int)ret); } static intmax_t -- 2.44.0