diff options
author | Noah Misch | 2015-05-18 14:02:31 +0000 |
---|---|---|
committer | Noah Misch | 2015-05-18 14:02:31 +0000 |
commit | 16304a013432931e61e623c8d85e9fe24709d9ba (patch) | |
tree | 8744b71d730def0f51abca8d488a4ddc6014144c /src/port/snprintf.c | |
parent | cac18a76bb6b08f1ecc2a85e46c9d2ab82dd9d23 (diff) |
Add error-throwing wrappers for the printf family of functions.
All known standard library implementations of these functions can fail
with ENOMEM. A caller neglecting to check for failure would experience
missing output, information exposure, or a crash. Check return values
within wrappers and code, currently just snprintf.c, that bypasses the
wrappers. The wrappers do not return after an error, so their callers
need not check. Back-patch to 9.0 (all supported versions).
Popular free software standard library implementations do take pains to
bypass malloc() in simple cases, but they risk ENOMEM for floating point
numbers, positional arguments, large field widths, and large precisions.
No specification demands such caution, so this commit regards every call
to a printf family function as a potential threat.
Injecting the wrappers implicitly is a compromise between patch scope
and design goals. I would prefer to edit each call site to name a
wrapper explicitly. libpq and the ECPG libraries would, ideally, convey
errors to the caller rather than abort(). All that would be painfully
invasive for a back-patched security fix, hence this compromise.
Security: CVE-2015-3166
Diffstat (limited to 'src/port/snprintf.c')
-rw-r--r-- | src/port/snprintf.c | 94 |
1 files changed, 55 insertions, 39 deletions
diff --git a/src/port/snprintf.c b/src/port/snprintf.c index 0c779a601fc..91c97d487cd 100644 --- a/src/port/snprintf.c +++ b/src/port/snprintf.c @@ -114,6 +114,7 @@ typedef struct /* bufend == NULL is for sprintf, where we assume buf is big enough */ FILE *stream; /* eventual output destination, or NULL */ int nchars; /* # chars already sent to stream */ + bool failed; /* call is a failure; errno is set */ } PrintfTarget; /* @@ -143,7 +144,7 @@ typedef union static void flushbuffer(PrintfTarget *target); -static int dopr(PrintfTarget *target, const char *format, va_list args); +static void dopr(PrintfTarget *target, const char *format, va_list args); int @@ -157,14 +158,10 @@ pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args) target.bufend = str + count - 1; target.stream = NULL; /* target.nchars is unused in this case */ - if (dopr(&target, fmt, args)) - { - *(target.bufptr) = '\0'; - errno = EINVAL; /* bad format */ - return -1; - } + target.failed = false; + dopr(&target, fmt, args); *(target.bufptr) = '\0'; - return target.bufptr - target.bufstart; + return target.failed ? -1 : (target.bufptr - target.bufstart); } int @@ -190,14 +187,10 @@ pg_vsprintf(char *str, const char *fmt, va_list args) target.bufend = NULL; target.stream = NULL; /* target.nchars is unused in this case */ - if (dopr(&target, fmt, args)) - { - *(target.bufptr) = '\0'; - errno = EINVAL; /* bad format */ - return -1; - } + target.failed = false; + dopr(&target, fmt, args); *(target.bufptr) = '\0'; - return target.bufptr - target.bufstart; + return target.failed ? -1 : (target.bufptr - target.bufstart); } int @@ -227,14 +220,11 @@ pg_vfprintf(FILE *stream, const char *fmt, va_list args) target.bufend = buffer + sizeof(buffer) - 1; target.stream = stream; target.nchars = 0; - if (dopr(&target, fmt, args)) - { - errno = EINVAL; /* bad format */ - return -1; - } + target.failed = false; + dopr(&target, fmt, args); /* dump any remaining buffer contents */ flushbuffer(&target); - return target.nchars; + return target.failed ? -1 : target.nchars; } int @@ -261,14 +251,24 @@ pg_printf(const char *fmt,...) return len; } -/* call this only when stream is defined */ +/* + * Attempt to write the entire buffer to target->stream; discard the entire + * buffer in any case. Call this only when target->stream is defined. + */ static void flushbuffer(PrintfTarget *target) { size_t nc = target->bufptr - target->bufstart; - if (nc > 0) - target->nchars += fwrite(target->bufstart, 1, nc, target->stream); + if (!target->failed && nc > 0) + { + size_t written; + + written = fwrite(target->bufstart, 1, nc, target->stream); + target->nchars += written; + if (written != nc) + target->failed = true; + } target->bufptr = target->bufstart; } @@ -295,7 +295,7 @@ static void trailing_pad(int *padlen, PrintfTarget *target); /* * dopr(): poor man's version of doprintf */ -static int +static void dopr(PrintfTarget *target, const char *format, va_list args) { const char *format_start = format; @@ -372,12 +372,12 @@ nextch1: case '$': have_dollar = true; if (accum <= 0 || accum > NL_ARGMAX) - return -1; + goto bad_format; if (afterstar) { if (argtypes[accum] && argtypes[accum] != ATYPE_INT) - return -1; + goto bad_format; argtypes[accum] = ATYPE_INT; last_dollar = Max(last_dollar, accum); afterstar = false; @@ -427,7 +427,7 @@ nextch1: atype = ATYPE_INT; if (argtypes[fmtpos] && argtypes[fmtpos] != atype) - return -1; + goto bad_format; argtypes[fmtpos] = atype; last_dollar = Max(last_dollar, fmtpos); } @@ -439,7 +439,7 @@ nextch1: { if (argtypes[fmtpos] && argtypes[fmtpos] != ATYPE_INT) - return -1; + goto bad_format; argtypes[fmtpos] = ATYPE_INT; last_dollar = Max(last_dollar, fmtpos); } @@ -452,7 +452,7 @@ nextch1: { if (argtypes[fmtpos] && argtypes[fmtpos] != ATYPE_CHARPTR) - return -1; + goto bad_format; argtypes[fmtpos] = ATYPE_CHARPTR; last_dollar = Max(last_dollar, fmtpos); } @@ -468,7 +468,7 @@ nextch1: { if (argtypes[fmtpos] && argtypes[fmtpos] != ATYPE_DOUBLE) - return -1; + goto bad_format; argtypes[fmtpos] = ATYPE_DOUBLE; last_dollar = Max(last_dollar, fmtpos); } @@ -489,7 +489,7 @@ nextch1: /* Per spec, you use either all dollar or all not. */ if (have_dollar && have_non_dollar) - return -1; + goto bad_format; /* * In dollar mode, collect the arguments in physical order. @@ -499,7 +499,7 @@ nextch1: switch (argtypes[i]) { case ATYPE_NONE: - return -1; /* invalid format */ + goto bad_format; case ATYPE_INT: argvalues[i].i = va_arg(args, int); break; @@ -524,6 +524,9 @@ nextch1: format = format_start; while ((ch = *format++) != '\0') { + if (target->failed) + break; + if (ch != '%') { dopr_outch(ch, target); @@ -781,7 +784,11 @@ nextch2: } } - return 0; + return; + +bad_format: + errno = EINVAL; + target->failed = true; } static size_t @@ -831,8 +838,10 @@ fmtptr(void *value, PrintfTarget *target) /* we rely on regular C library's sprintf to do the basic conversion */ vallen = sprintf(convert, "%p", value); - - dostr(convert, vallen, target); + if (vallen < 0) + target->failed = true; + else + dostr(convert, vallen, target); } static void @@ -965,16 +974,19 @@ fmtfloat(double value, char type, int forcesign, int leftjust, if (pointflag) { - sprintf(fmt, "%%.%d%c", prec, type); + if (sprintf(fmt, "%%.%d%c", prec, type) < 0) + goto fail; zeropadlen = precision - prec; } - else - sprintf(fmt, "%%%c", type); + else if (sprintf(fmt, "%%%c", type) < 0) + goto fail; if (!isnan(value) && adjust_sign((value < 0), forcesign, &signvalue)) value = -value; vallen = sprintf(convert, fmt, value); + if (vallen < 0) + goto fail; /* If it's infinity or NaN, forget about doing any zero-padding */ if (zeropadlen > 0 && !isdigit((unsigned char) convert[vallen - 1])) @@ -1014,6 +1026,10 @@ fmtfloat(double value, char type, int forcesign, int leftjust, } trailing_pad(&padlen, target); + return; + +fail: + target->failed = true; } static void |