From aa6a183700ba7afec42534fcc7b4b0b2b5c89891 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Tue, 12 Mar 2024 12:27:48 -0700 Subject: [PATCH 4/4] printf now diagnoses width or prec overflow * printf.def: Include stdckdint.h. (skipint, scanint): New functions, replacing decodeprec. 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 | 147 +++++++++++++++++++++----------------------- 1 file changed, 69 insertions(+), 78 deletions(-) diff --git a/builtins/printf.def b/builtins/printf.def index f711ed38..c801ec10 100644 --- a/builtins/printf.def +++ b/builtins/printf.def @@ -69,7 +69,7 @@ $END #endif #include - +#include #include #include @@ -200,7 +200,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); @@ -244,22 +244,55 @@ static intmax_t tw; static char *conv_buf; static size_t conv_bufsize; +/* Skip the unsigned decimal integer prefix, if any, of STR, + and return the address of the first byte after the prefix. + If DIAGNOSE< diagnose if the integer would not fit in int. */ +static char * +skipint (char *str, int diagnose) +{ + char *ps = str; + int pr = 0; + int v = 0; + + 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); + } + return ps; +} + +/* Convert the unsigned decimal integer at *STR to int. + **STR must be a digit. + Update *STR to point past the end of the integer. + Return the scanned value, or IFOVERFLOW if it would not fit in int. */ static inline int -decodeprec (char *ps) +scanint (char **str, 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'); + } + *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*/ @@ -330,6 +363,7 @@ printf_builtin (WORD_LIST *list) format = list->word->word; tw = 0; + diagnose = 1; retval = EXECUTION_SUCCESS; garglist = orig_arglist = list->next; @@ -407,14 +441,10 @@ 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)) - fmt++; + fmt = skipint (fmt, diagnose); /* Skip optional '.' and precision */ if (*fmt == '.') @@ -424,11 +454,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 { @@ -443,9 +469,10 @@ printf_builtin (WORD_LIST *list) #endif fmt++; if (DIGIT (*fmt)) - precstart = fmt; - while (DIGIT (*fmt)) - fmt++; + { + precstart = fmt; + fmt = skipint (fmt, diagnose); + } } } @@ -660,7 +687,10 @@ printf_builtin (WORD_LIST *list) if (convch == 'Q' && (have_precision || precstart)) { if (precstart) - precision = decodeprec (precstart); + { + char *prec = precstart; + precision = scanint (&prec, -1); + } slen = strlen (p); /* printf precision works in bytes. */ if (precision >= 0 && precision < slen) @@ -700,8 +730,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); @@ -712,6 +742,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); } @@ -790,6 +821,8 @@ printf_builtin (WORD_LIST *list) /* PRETURN will print error message. */ PRETURN (EXECUTION_FAILURE); } + + diagnose = 0; } while (garglist && garglist != list->next); @@ -823,7 +856,6 @@ printstr (char *fmt, char *string, int len, int fieldwidth, int precision) #endif int padlen, nc, ljust, i; int fw, pr; /* fieldwidth and precision */ - intmax_t mfw, mpr; if (string == 0) string = ""; @@ -834,10 +866,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)) @@ -859,13 +889,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 = scanint (&fmt, 0); /* get precision, if present. doesn't handle negative precisions */ if (*fmt == '.') @@ -877,15 +901,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 = scanint (&fmt, -1); else pr = 0; /* "a null digit string is treated as zero" */ } @@ -934,7 +950,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""; @@ -947,8 +962,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)) @@ -970,13 +983,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 = scanint (&fmt, 0); /* get precision, if present. doesn't handle negative precisions */ if (*fmt == '.') @@ -988,15 +995,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 = scanint (&fmt, -1); else pr = 0; /* "a null digit string is treated as zero" */ } @@ -1346,10 +1345,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); @@ -1359,27 +1359,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