From f2b6ebed7bfcfcbe65bcadbce1c5ae7e6134c11e Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 10 Jul 2012 23:11:30 +0200 Subject: [PATCH] cmdline: parse numerical options stricter 1 - str2offset() no longer accepts negative numbers since offsets are by nature positive. 2 - introduced str2unum() for the command line parser that accepts numericals which are not supposed to be negative, so that it will properly complain on apparent bad uses and mistakes. Bug: http://curl.haxx.se/mail/archive-2012-07/0013.html --- src/tool_getparam.c | 62 ++++++++++++++++++++++++++++----------------- src/tool_getparam.h | 1 + src/tool_helpers.c | 2 ++ src/tool_paramhlp.c | 46 +++++++++++++++++++++++++-------- src/tool_paramhlp.h | 1 + 5 files changed, 78 insertions(+), 34 deletions(-) diff --git a/src/tool_getparam.c b/src/tool_getparam.c index 4205f596a..579437eb7 100644 --- a/src/tool_getparam.c +++ b/src/tool_getparam.c @@ -397,8 +397,9 @@ ParameterError getparameter(char *flag, /* f or -long-flag */ GetStr(&config->egd_file, nextarg); break; case 'c': /* connect-timeout */ - if(str2num(&config->connecttimeout, nextarg)) - return PARAM_BAD_NUMERIC; + rc=str2unum(&config->connecttimeout, nextarg); + if(rc) + return rc; break; case 'd': /* ciphers */ GetStr(&config->cipher_list, nextarg); @@ -541,8 +542,12 @@ ParameterError getparameter(char *flag, /* f or -long-flag */ break; case 's': /* --max-redirs */ - /* specified max no of redirects (http(s)) */ - if(str2num(&config->maxredirs, nextarg)) + /* specified max no of redirects (http(s)), this accepts -1 as a + special condition */ + rc = str2num(&config->maxredirs, nextarg); + if(rc) + return rc; + if(config->maxredirs < -1) return PARAM_BAD_NUMERIC; break; @@ -586,8 +591,9 @@ ParameterError getparameter(char *flag, /* f or -long-flag */ return PARAM_LIBCURL_DOESNT_SUPPORT; break; case 'y': /* --max-filesize */ - if(str2offset(&config->max_filesize, nextarg)) - return PARAM_BAD_NUMERIC; + rc = str2offset(&config->max_filesize, nextarg); + if(rc) + return rc; break; case 'z': /* --disable-eprt */ config->disable_eprt = toggle; @@ -663,16 +669,19 @@ ParameterError getparameter(char *flag, /* f or -long-flag */ config->proxybasic = toggle; break; case 'g': /* --retry */ - if(str2num(&config->req_retry, nextarg)) - return PARAM_BAD_NUMERIC; + rc = str2unum(&config->req_retry, nextarg); + if(rc) + return rc; break; case 'h': /* --retry-delay */ - if(str2num(&config->retry_delay, nextarg)) - return PARAM_BAD_NUMERIC; + rc = str2unum(&config->retry_delay, nextarg); + if(rc) + return rc; break; case 'i': /* --retry-max-time */ - if(str2num(&config->retry_maxtime, nextarg)) - return PARAM_BAD_NUMERIC; + rc = str2unum(&config->retry_maxtime, nextarg); + if(rc) + return rc; break; case 'k': /* --proxy-negotiate */ @@ -759,8 +768,9 @@ ParameterError getparameter(char *flag, /* f or -long-flag */ config->nokeepalive = (!toggle)?TRUE:FALSE; break; case '3': /* --keepalive-time */ - if(str2num(&config->alivetime, nextarg)) - return PARAM_BAD_NUMERIC; + rc = str2unum(&config->alivetime, nextarg); + if(rc) + return rc; break; case '4': /* --post302 */ config->post302 = toggle; @@ -786,7 +796,9 @@ ParameterError getparameter(char *flag, /* f or -long-flag */ config->proxyver = CURLPROXY_HTTP_1_0; break; case '9': /* --tftp-blksize */ - str2num(&config->tftp_blksize, nextarg); + rc = str2unum(&config->tftp_blksize, nextarg); + if(rc) + return rc; break; case 'A': /* --mail-from */ GetStr(&config->mail_from, nextarg); @@ -911,8 +923,9 @@ ParameterError getparameter(char *flag, /* f or -long-flag */ case 'C': /* This makes us continue an ftp transfer at given position */ if(!curlx_strequal(nextarg, "-")) { - if(str2offset(&config->resume_from, nextarg)) - return PARAM_BAD_NUMERIC; + rc = str2offset(&config->resume_from, nextarg); + if(rc) + return rc; config->resume_from_current = FALSE; } else { @@ -1303,8 +1316,9 @@ ParameterError getparameter(char *flag, /* f or -long-flag */ break; case 'm': /* specified max time */ - if(str2num(&config->timeout, nextarg)) - return PARAM_BAD_NUMERIC; + rc = str2unum(&config->timeout, nextarg); + if(rc) + return rc; break; case 'M': /* M for manual, huge help */ if(toggle) { /* --no-manual shows no manual... */ @@ -1618,15 +1632,17 @@ ParameterError getparameter(char *flag, /* f or -long-flag */ break; case 'y': /* low speed time */ - if(str2num(&config->low_speed_time, nextarg)) - return PARAM_BAD_NUMERIC; + rc = str2unum(&config->low_speed_time, nextarg); + if(rc) + return rc; if(!config->low_speed_limit) config->low_speed_limit = 1; break; case 'Y': /* low speed limit */ - if(str2num(&config->low_speed_limit, nextarg)) - return PARAM_BAD_NUMERIC; + rc = str2unum(&config->low_speed_limit, nextarg); + if(rc) + return rc; if(!config->low_speed_time) config->low_speed_time = 30; break; diff --git a/src/tool_getparam.h b/src/tool_getparam.h index 7dc49393b..49cc684c9 100644 --- a/src/tool_getparam.h +++ b/src/tool_getparam.h @@ -32,6 +32,7 @@ typedef enum { PARAM_HELP_REQUESTED, PARAM_GOT_EXTRA_PARAMETER, PARAM_BAD_NUMERIC, + PARAM_NEGATIVE_NUMERIC, PARAM_LIBCURL_DOESNT_SUPPORT, PARAM_NO_MEM, PARAM_LAST diff --git a/src/tool_helpers.c b/src/tool_helpers.c index 87d7c609d..ae8aaaf32 100644 --- a/src/tool_helpers.c +++ b/src/tool_helpers.c @@ -54,6 +54,8 @@ const char *param2text(int res) return "is badly used here"; case PARAM_BAD_NUMERIC: return "expected a proper numerical parameter"; + case PARAM_NEGATIVE_NUMERIC: + return "expected a positive numerical parameter"; case PARAM_LIBCURL_DOESNT_SUPPORT: return "the installed libcurl version doesn't support this"; case PARAM_NO_MEM: diff --git a/src/tool_paramhlp.c b/src/tool_paramhlp.c index 4d1d17a52..85912a2ef 100644 --- a/src/tool_paramhlp.c +++ b/src/tool_paramhlp.c @@ -146,8 +146,8 @@ void cleanarg(char *str) } /* - * Parse the string and write the integer in the given address. Return - * non-zero on failure, zero on success. + * Parse the string and write the long in the given address. Return non-zero + * on failure, zero on success. * * Since this function gets called with the 'nextarg' pointer from within the * getparameter a lot, we must check it for NULL before accessing the str @@ -161,10 +161,26 @@ int str2num(long *val, const char *str) long num = strtol(str, &endptr, 10); if((endptr != str) && (endptr == str + strlen(str))) { *val = num; - return 0; /* Ok */ + return PARAM_OK; /* Ok */ } } - return 1; /* badness */ + return PARAM_BAD_NUMERIC; /* badness */ +} + +/* + * Parse the string and write the long in the given address. Return non-zero + * on failure, zero on success. ONLY ACCEPTS POSITIVE NUMBERS! + * + * Since this function gets called with the 'nextarg' pointer from within the + * getparameter a lot, we must check it for NULL before accessing the str + * data. + */ + +int str2unum(long *val, const char *str) +{ + if(str[0]=='-') + return PARAM_NEGATIVE_NUMERIC; /* badness */ + return str2num(val, str); } /* @@ -274,8 +290,8 @@ long proto2num(struct Configurable *config, long *val, const char *str) } /** - * Parses the given string looking for an offset (which may be - * a larger-than-integer value). + * Parses the given string looking for an offset (which may be a + * larger-than-integer value). The offset CANNOT be negative! * * @param val the offset to populate * @param str the buffer containing the offset @@ -283,16 +299,24 @@ long proto2num(struct Configurable *config, long *val, const char *str) */ int str2offset(curl_off_t *val, const char *str) { + char *endptr; + if(str[0] == '-') + /* offsets aren't negative, this indicates weird input */ + return PARAM_NEGATIVE_NUMERIC; + #if(CURL_SIZEOF_CURL_OFF_T > CURL_SIZEOF_LONG) - *val = curlx_strtoofft(str, NULL, 0); + *val = curlx_strtoofft(str, &endptr, 0); if((*val == CURL_OFF_T_MAX || *val == CURL_OFF_T_MIN) && (ERRNO == ERANGE)) - return 1; + return PARAM_BAD_NUMERIC; #else - *val = strtol(str, NULL, 0); + *val = strtol(str, &endptr, 0); if((*val == LONG_MIN || *val == LONG_MAX) && ERRNO == ERANGE) - return 1; + return PARAM_BAD_NUMERIC; #endif - return 0; + if((endptr != str) && (endptr == str + strlen(str))) + return 0; /* Ok */ + + return PARAM_BAD_NUMERIC; } ParameterError checkpasswd(const char *kind, /* for what purpose */ diff --git a/src/tool_paramhlp.h b/src/tool_paramhlp.h index c65450915..50c693efd 100644 --- a/src/tool_paramhlp.h +++ b/src/tool_paramhlp.h @@ -32,6 +32,7 @@ ParameterError file2memory(char **bufp, size_t *size, FILE *file); void cleanarg(char *str); int str2num(long *val, const char *str); +int str2unum(long *val, const char *str); /* for unsigned input numbers */ long proto2num(struct Configurable *config, long *val, const char *str);