From f15a88f2b25ee44d8c8d3bdcf2508fdf50f8b868 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Fri, 16 Aug 2013 11:52:59 +0200 Subject: [PATCH] glob: error out on range overflow The new multiply() function detects range value overflows. 32bit machines will overflow on a 32bit boundary while 64bit hosts support ranges up to the full 64 bit range. Added test 1236 to verify. Bug: http://curl.haxx.se/bug/view.cgi?id=1267 Reported-by: Will Dietz --- src/tool_urlglob.c | 34 ++++++++++++++++++++++++++++------ tests/data/Makefile.am | 1 + tests/data/test1236 | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 tests/data/test1236 diff --git a/src/tool_urlglob.c b/src/tool_urlglob.c index 0e454c19c..ac13d683d 100644 --- a/src/tool_urlglob.c +++ b/src/tool_urlglob.c @@ -62,6 +62,19 @@ static GlobCode glob_fixed(URLGlob *glob, unsigned long *amount) return GLOB_OK; } +/* multiply + * + * Multiplies and checks for overflow. + */ +static int multiply(unsigned long *amount, long with) +{ + unsigned long sum = *amount * with; + if(sum/with != *amount) + return 1; /* didn't fit, bail out */ + *amount = sum; + return 0; +} + static GlobCode glob_set(URLGlob *glob, char **patternp, size_t pos, unsigned long *amount, int globindex) @@ -102,8 +115,11 @@ static GlobCode glob_set(URLGlob *glob, char **patternp, "no string within braces at pos %zu\n", pos); return GLOB_ERROR; } - /* add 1 since it'll be incremented below */ - (*amount) *= (pat->content.Set.size+1); + /* add 1 to size since it'll be incremented below */ + if(multiply(amount, pat->content.Set.size+1)) { + strcpy(glob->errormsg, "range overflow!\n"); + return GLOB_ERROR; + } /* fall-through */ case ',': @@ -224,8 +240,11 @@ static GlobCode glob_range(URLGlob *glob, char **patternp, pat->content.CharRange.ptr_c = pat->content.CharRange.min_c = min_c; pat->content.CharRange.max_c = max_c; - *amount *= (pat->content.CharRange.max_c - - pat->content.CharRange.min_c + 1); + if(multiply(amount, (pat->content.CharRange.max_c - + pat->content.CharRange.min_c + 1))) { + strcpy(glob->errormsg, "range overflow!\n"); + return GLOB_ERROR; + } } else if(ISDIGIT(*pattern)) { /* numeric range detected */ @@ -288,8 +307,11 @@ static GlobCode glob_range(URLGlob *glob, char **patternp, pat->content.NumRange.max_n = max_n; pat->content.NumRange.step = step_n; - *amount *= (pat->content.NumRange.max_n - - pat->content.NumRange.min_n + 1); + if(multiply(amount, (pat->content.NumRange.max_n - + pat->content.NumRange.min_n + 1))) { + strcpy(glob->errormsg, "range overflow!\n"); + return GLOB_ERROR; + } } else { snprintf(glob->errormsg, sizeof(glob->errormsg), diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index f661b7103..dd7f6f95d 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -94,6 +94,7 @@ test1208 test1209 test1210 test1211 test1212 test1213 test1214 test1215 \ test1216 test1217 test1218 test1219 \ test1220 test1221 test1222 test1223 test1224 test1225 test1226 test1227 \ test1228 test1229 test1230 test1231 test1232 test1233 test1234 test1235 \ +test1236 \ \ test1300 test1301 test1302 test1303 test1304 test1305 test1306 test1307 \ test1308 test1309 test1310 test1311 test1312 test1313 test1314 test1315 \ diff --git a/tests/data/test1236 b/tests/data/test1236 new file mode 100644 index 000000000..0829be313 --- /dev/null +++ b/tests/data/test1236 @@ -0,0 +1,33 @@ + + + +globbing +FAILURE + + +# Server-side + + + +# Client-side + + +none + + +[] globbing overflowing the range counter + +# 2^62 == 4611686018427387904 + +"%HOSTIP:%HTTPPORT/1234[0-1]{" "%HOSTIP:%HTTPPORT/[1-4611686018427387904][1-4611686018427387904]" + + + +# Verify data after the test has been "shot" + +# 3 == CURLE_URL_MALFORMAT + +3 + + +