diff --git a/include/httpd.h b/include/httpd.h index 81ac0e80f48..8c9d24b6c6b 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -2364,13 +2364,16 @@ AP_DECLARE(apr_status_t) ap_timeout_parameter_parse( const char *default_time_unit); /** - * Parse a configuration parameter as a base 10 apr_off_t. The whole parameter - * has to be a number, there must not be any other character after it. What + * Parse a configuration parameter as a base 10 apr_off_t. The number may be + * followed by a single optional suffix, 'b'/'B' (bytes), 'k'/'K' (multiply + * by 1024), 'm'/'M' (multiply by 1024*1024) or 'g'/'G' (multiply by + * 1024*1024*1024); there must not be any other character after it. What * values are acceptable is up to the caller. * @param parameter The string containing the number. * @param value The parsed value, only set on success. - * @return APR_SUCCESS, APR_EINVAL if the parameter is not a number, or - * APR_ERANGE if the number does not fit in an apr_off_t. + * @return APR_SUCCESS, APR_EINVAL if the parameter is not a number with + * optional suffix, or APR_ERANGE if the value does not fit in an + * apr_off_t. */ AP_DECLARE(apr_status_t) ap_size_parameter_parse(const char *parameter, apr_off_t *value); diff --git a/modules/aaa/mod_auth_digest.c b/modules/aaa/mod_auth_digest.c index 9188c7252cf..3c8b1a9b0d5 100644 --- a/modules/aaa/mod_auth_digest.c +++ b/modules/aaa/mod_auth_digest.c @@ -555,35 +555,15 @@ static const char *set_uri_list(cmd_parms *cmd, void *config, const char *uri) static const char *set_shmem_size(cmd_parms *cmd, void *config, const char *size_str) { - char *endptr; apr_off_t size; apr_size_t min; + apr_status_t rv; - if (apr_strtoff(&size, size_str, &endptr, 10) != APR_SUCCESS || size < 0) { - return apr_pstrcat(cmd->pool, "Invalid size in AuthDigestShmemSize: ", - size_str, NULL); - } - if (*endptr == 'k' || *endptr == 'K') { - if (size > APR_INT64_MAX / 1024) { - return "AuthDigestShmemSize value is too large"; - } - size *= 1024; - endptr++; - } - else if (*endptr == 'm' || *endptr == 'M') { - if (size > APR_INT64_MAX / (1024 * 1024)) { - return "AuthDigestShmemSize value is too large"; - } - size *= 1024 * 1024; - endptr++; - } - else if (*endptr == 'b' || *endptr == 'B') { - endptr++; - } - while (apr_isspace(*endptr)) { - endptr++; + rv = ap_size_parameter_parse(size_str, &size); + if (rv == APR_ERANGE) { + return "AuthDigestShmemSize value is too large"; } - if (*endptr != '\0') { + if (rv != APR_SUCCESS || size < 0) { return apr_pstrcat(cmd->pool, "Invalid size in AuthDigestShmemSize: ", size_str, NULL); } diff --git a/modules/filters/mod_substitute.c b/modules/filters/mod_substitute.c index c78992c4833..6cb9fa06ba4 100644 --- a/modules/filters/mod_substitute.c +++ b/modules/filters/mod_substitute.c @@ -773,40 +773,17 @@ static const char *set_pattern(cmd_parms *cmd, void *cfg, const char *line) return NULL; } -#define KBYTE 1024 -#define MBYTE 1048576 -#define GBYTE 1073741824 static const char *set_max_line_length(cmd_parms *cmd, void *cfg, const char *arg) { subst_dir_conf *dcfg = (subst_dir_conf *)cfg; apr_off_t max; - char *end; apr_status_t rv; - rv = apr_strtoff(&max, arg, &end, 10); - if (rv == APR_SUCCESS) { - if ((*end == 'K' || *end == 'k') && !end[1]) { - if (max > APR_INT64_MAX / KBYTE) - return "SubstituteMaxLineLength value too large"; - max *= KBYTE; - } - else if ((*end == 'M' || *end == 'm') && !end[1]) { - if (max > APR_INT64_MAX / MBYTE) - return "SubstituteMaxLineLength value too large"; - max *= MBYTE; - } - else if ((*end == 'G' || *end == 'g') && !end[1]) { - if (max > APR_INT64_MAX / GBYTE) - return "SubstituteMaxLineLength value too large"; - max *= GBYTE; - } - else if (*end && /* neither empty nor [Bb] */ - ((*end != 'B' && *end != 'b') || end[1])) { - rv = APR_EGENERAL; - } + rv = ap_size_parameter_parse(arg, &max); + if (rv == APR_ERANGE) { + return "SubstituteMaxLineLength value too large"; } - if (rv != APR_SUCCESS || max < 0) { return "SubstituteMaxLineLength must be a non-negative integer optionally " diff --git a/server/util.c b/server/util.c index 987e9d48207..471caeb6a66 100644 --- a/server/util.c +++ b/server/util.c @@ -2777,7 +2777,10 @@ AP_DECLARE(apr_status_t) ap_timeout_parameter_parse( AP_DECLARE(apr_status_t) ap_size_parameter_parse(const char *parameter, apr_off_t *value) { - apr_off_t parsed; + /* The maximum apr_off_t value. */ + const apr_off_t off_max = sizeof(apr_off_t) == 8 ? APR_INT64_MAX + : APR_INT32_MAX; + apr_off_t parsed, multiplier = 1; apr_status_t rv; char *end; @@ -2790,11 +2793,32 @@ AP_DECLARE(apr_status_t) ap_size_parameter_parse(const char *parameter, if (rv != APR_SUCCESS) { return APR_ERANGE; } + + if (*end == 'k' || *end == 'K') { + multiplier = 1024; + end++; + } + else if (*end == 'm' || *end == 'M') { + multiplier = 1024 * 1024; + end++; + } + else if (*end == 'g' || *end == 'G') { + multiplier = 1024 * 1024 * 1024; + end++; + } + else if (*end == 'b' || *end == 'B') { + end++; + } if (*end != '\0') { return APR_EINVAL; } - *value = parsed; + if (parsed > off_max / multiplier + || parsed < (-off_max - 1) / multiplier) { + return APR_ERANGE; + } + + *value = parsed * multiplier; return APR_SUCCESS; } #undef CHECK_OVERFLOW diff --git a/test/modules/core/test_012_config_off_t.py b/test/modules/core/test_012_config_off_t.py index 6e4d4a626eb..61577733793 100644 --- a/test/modules/core/test_012_config_off_t.py +++ b/test/modules/core/test_012_config_off_t.py @@ -28,7 +28,9 @@ def check_config(self, env, directive, value): "-f", os.path.join(env.server_dir, 'conf', 'httpd.conf')]) @pytest.mark.parametrize("directive", DIRECTIVES) - @pytest.mark.parametrize("value", ["0", "1", "123", "1000000", "00123"]) + @pytest.mark.parametrize("value", ["0", "1", "123", "1000000", "00123", + "512b", "512B", "1k", "1K", "2m", "2M", + "1g", "1G"]) def test_core_012_001(self, env, directive, value): r = self.check_config(env, directive, value) assert r.exit_code == 0, f"{r.stderr}" @@ -36,7 +38,8 @@ def test_core_012_001(self, env, directive, value): # only a whole decimal number is valid @pytest.mark.parametrize("directive", DIRECTIVES) @pytest.mark.parametrize("value", [ - "abc", "abc123", "123abc", "123foo456", "12.3", "0x10", "1K", "+", "-", + "abc", "abc123", "123abc", "123foo456", "12.3", "0x10", "1kb", "k", + "+", "-", ]) def test_core_012_002(self, env, directive, value): r = self.check_config(env, directive, value) @@ -45,7 +48,8 @@ def test_core_012_002(self, env, directive, value): # a number the directive does not accept, or too large to be stored @pytest.mark.parametrize("directive", DIRECTIVES) - @pytest.mark.parametrize("value", ["-1", "9223372036854775808", "9" * 40]) + @pytest.mark.parametrize("value", ["-1", "9223372036854775808", "9" * 40, + "8589934592g"]) def test_core_012_003(self, env, directive, value): r = self.check_config(env, directive, value) assert r.exit_code != 0, f"{value} was accepted" diff --git a/test/unit/util.c b/test/unit/util.c index 26f26ad82be..3cca7c2b19f 100644 --- a/test/unit/util.c +++ b/test/unit/util.c @@ -149,6 +149,75 @@ HTTPD_START_LOOP_TEST(check_escape_quotes, ap_test_escape_quotes_cases_len) END_TEST +/* + * ap_size_parameter_parse() + */ + +struct ap_size_parameter_parse_case { + const char *input; + apr_status_t expected_rv; + apr_int64_t expected; /* for a 64-bit apr_off_t */ +}; + +const struct ap_size_parameter_parse_case ap_size_parameter_parse_cases[] = { + { "0", APR_SUCCESS, 0 }, + { "1", APR_SUCCESS, 1 }, + { "-1", APR_SUCCESS, -1 }, + { "1024", APR_SUCCESS, 1024 }, + { "0b", APR_SUCCESS, 0 }, + { "1b", APR_SUCCESS, 1 }, + { "512B", APR_SUCCESS, 512 }, + { "1k", APR_SUCCESS, 1024 }, + { "1K", APR_SUCCESS, 1024 }, + { "10k", APR_SUCCESS, 10240 }, + { "1m", APR_SUCCESS, 1024 * 1024 }, + { "2M", APR_SUCCESS, 2 * 1024 * 1024 }, + { "1g", APR_SUCCESS, 1024 * 1024 * 1024 }, + { "4G", APR_SUCCESS, APR_INT64_C(4) * 1024 * 1024 * 1024 }, + { "-2k", APR_SUCCESS, -2048 }, + { "", APR_EINVAL, 0 }, + { "k", APR_EINVAL, 0 }, + { "-", APR_EINVAL, 0 }, + { "1t", APR_EINVAL, 0 }, + { "1kb", APR_EINVAL, 0 }, + { "1 k", APR_EINVAL, 0 }, + { "1k ", APR_EINVAL, 0 }, + { "2 ", APR_EINVAL, 0 }, + { "0x10", APR_EINVAL, 0 }, + { "1.5k", APR_EINVAL, 0 }, + /* overflows whether apr_off_t is 32 or 64 bits wide */ + { "99999999999999999999", APR_ERANGE, 0 }, + { "9223372036854775807k", APR_ERANGE, 0 }, + { "9007199254740992m", APR_ERANGE, 0 }, + { "-9007199254740992m", APR_ERANGE, 0 }, + { "8589934592g", APR_ERANGE, 0 }, +}; + +const size_t ap_size_parameter_parse_cases_len = + sizeof(ap_size_parameter_parse_cases) / + sizeof(ap_size_parameter_parse_cases[0]); + +HTTPD_START_LOOP_TEST(size_parameter_parse_handles_suffixes, ap_size_parameter_parse_cases_len) +{ + const struct ap_size_parameter_parse_case *c = &ap_size_parameter_parse_cases[_i]; + apr_status_t expected_rv = c->expected_rv; + apr_off_t value = 0; + apr_status_t rv; + + /* A value which does not fit a narrower apr_off_t is a range error. */ + if (expected_rv == APR_SUCCESS && (apr_off_t)c->expected != c->expected) { + expected_rv = APR_ERANGE; + } + + rv = ap_size_parameter_parse(c->input, &value); + ck_assert_int_eq(rv, expected_rv); + if (expected_rv == APR_SUCCESS) { + ck_assert_int_eq(value, (apr_off_t)c->expected); + } +} +END_TEST + + /* * Test Case Boilerplate */