Skip to content

Commit 7e32f65

Browse files
committed
mprintf: fix integer handling in float precision
In the double output function when an extremely large width and precision is set that reaches the libcurl maximum (325), the handling of the precision part would do wrong which could lead to bad output. Also: work-around for single-byte buffer snprintf overflow with mingw. Extend test 557 to verify. Coverity CID 1638751. Closes curl#15988
1 parent 97d278f commit 7e32f65

File tree

2 files changed

+59
-10
lines changed

2 files changed

+59
-10
lines changed

lib/mprintf.c

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -678,12 +678,12 @@ static int formatf(
678678

679679
struct outsegment output[MAX_SEGMENTS];
680680
struct va_input input[MAX_PARAMETERS];
681-
char work[BUFFSIZE];
681+
char work[BUFFSIZE + 2];
682682

683683
/* 'workend' points to the final buffer byte position, but with an extra
684684
byte as margin to avoid the (FALSE?) warning Coverity gives us
685685
otherwise */
686-
char *workend = &work[sizeof(work) - 2];
686+
char *workend = &work[BUFFSIZE - 2];
687687

688688
/* Parse the format string */
689689
if(parsefmt(format, output, input, &ocount, &icount, ap_save))
@@ -966,8 +966,8 @@ static int formatf(
966966

967967
if(width >= 0) {
968968
size_t dlen;
969-
if(width >= (int)sizeof(work))
970-
width = sizeof(work)-1;
969+
if(width >= BUFFSIZE)
970+
width = BUFFSIZE - 1;
971971
/* RECURSIVE USAGE */
972972
dlen = (size_t)curl_msnprintf(fptr, left, "%d", width);
973973
fptr += dlen;
@@ -976,17 +976,19 @@ static int formatf(
976976
if(prec >= 0) {
977977
/* for each digit in the integer part, we can have one less
978978
precision */
979-
size_t maxprec = sizeof(work) - 2;
979+
int maxprec = BUFFSIZE - 1;
980980
double val = iptr->val.dnum;
981+
if(prec > maxprec)
982+
prec = maxprec - 1;
981983
if(width > 0 && prec <= width)
982-
maxprec -= (size_t)width;
984+
maxprec -= width;
983985
while(val >= 10.0) {
984986
val /= 10;
985987
maxprec--;
986988
}
987989

988-
if(prec > (int)maxprec)
989-
prec = (int)maxprec-1;
990+
if(prec > maxprec)
991+
prec = maxprec - 1;
990992
if(prec < 0)
991993
prec = 0;
992994
/* RECURSIVE USAGE */
@@ -1012,14 +1014,19 @@ static int formatf(
10121014
/* NOTE NOTE NOTE!! Not all sprintf implementations return number of
10131015
output characters */
10141016
#ifdef HAVE_SNPRINTF
1015-
(snprintf)(work, sizeof(work), formatbuf, iptr->val.dnum); /* NOLINT */
1017+
(snprintf)(work, BUFFSIZE, formatbuf, iptr->val.dnum); /* NOLINT */
10161018
#else
10171019
(sprintf)(work, formatbuf, iptr->val.dnum);
10181020
#endif
10191021
#ifdef __clang__
10201022
#pragma clang diagnostic pop
10211023
#endif
1022-
DEBUGASSERT(strlen(work) <= sizeof(work));
1024+
DEBUGASSERT(strlen(work) < BUFFSIZE);
1025+
#ifdef __MINGW32__
1026+
/* Work-around for a nasty bug seen with old-mingw and gcc 7.3.0 when it
1027+
writes one byte more than permitted. */
1028+
work[BUFFSIZE - 1] = 0;
1029+
#endif
10231030
for(fptr = work; *fptr; fptr++)
10241031
OUTCHAR(*fptr);
10251032
break;

tests/libtest/lib557.c

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,46 @@ static int test_pos_arguments(void)
12071207
return errors;
12081208
}
12091209

1210+
static int test_width_precision(void)
1211+
{
1212+
/* 325 is max precision (and width) for a double */
1213+
char larger[1024];
1214+
#define SPACE60 " "
1215+
#define SPACE300 SPACE60 SPACE60 SPACE60 SPACE60 SPACE60
1216+
#define OK325 SPACE300 " 0"
1217+
1218+
int rc;
1219+
int errors = 0;
1220+
rc = curl_msnprintf(larger, sizeof(larger), "%325.325f", 0.1);
1221+
if(rc != 325)
1222+
errors++;
1223+
errors += string_check(larger, OK325);
1224+
1225+
rc = curl_msnprintf(larger, sizeof(larger), "%326.326f", 0.1);
1226+
if(rc != 325)
1227+
errors++;
1228+
errors += string_check(larger, OK325);
1229+
1230+
rc = curl_msnprintf(larger, sizeof(larger), "%1000.1000f", 0.1);
1231+
if(rc != 325)
1232+
errors++;
1233+
errors += string_check(larger, OK325);
1234+
1235+
rc = curl_msnprintf(larger, sizeof(larger), "%324.324f", 0.1);
1236+
if(rc != 324)
1237+
errors++;
1238+
rc = curl_msnprintf(larger, sizeof(larger), "%324.0f", 0.1);
1239+
if(rc != 324)
1240+
errors++;
1241+
rc = curl_msnprintf(larger, sizeof(larger), "%0.324f", 0.1);
1242+
if(rc != 325)
1243+
errors++;
1244+
1245+
return errors;
1246+
}
1247+
1248+
1249+
12101250
static int test_weird_arguments(void)
12111251
{
12121252
int errors = 0;
@@ -1320,6 +1360,8 @@ static int test_weird_arguments(void)
13201360

13211361
errors += string_check(buf, "");
13221362

1363+
errors += test_width_precision();
1364+
13231365
if(errors)
13241366
printf("Some curl_mprintf() weird arguments tests failed!\n");
13251367

0 commit comments

Comments
 (0)