From 1e7c96009a358be4f66de90cfe3e730d7c81f54f Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Mon, 16 Dec 2024 21:21:04 +0000 Subject: [PATCH] Add wrapper for strtoul(,,16) for safely parsing hex strings: * src/ne_string.c (ne_strhextoul): New function. * test/string-tests.c (strhextoul): Add test case. * src/ne_request.c (read_response_block): Use ne_strhextoul() rather than strtoul. * src/ne_auth.c (verify_digest_response): Likewise. * src/neon.vers, src/ne_string.h: Add new API. --- src/ne_auth.c | 5 ++-- src/ne_request.c | 19 +++++---------- src/ne_string.c | 23 +++++++++++++++++++ src/ne_string.h | 7 ++++++ src/neon.vers | 3 +++ test/request.c | 2 ++ test/string-tests.c | 56 +++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 99 insertions(+), 16 deletions(-) diff --git a/src/ne_auth.c b/src/ne_auth.c index eb61586e..76d2f27d 100644 --- a/src/ne_auth.c +++ b/src/ne_auth.c @@ -1305,10 +1305,9 @@ static int verify_digest_response(struct auth_request *req, auth_session *sess, "client nonce mismatch")); } else if (nc) { - char *ptr; + const char *ptr; - errno = 0; - nonce_count = strtoul(nc, &ptr, 16); + nonce_count = ne_strhextoul(nc, &ptr); if (*ptr != '\0' || errno) { ret = NE_ERROR; ne_set_error(sess->sess, _("Digest mutual authentication failure: " diff --git a/src/ne_request.c b/src/ne_request.c index c55316c1..3ad7b861 100644 --- a/src/ne_request.c +++ b/src/ne_request.c @@ -935,6 +935,7 @@ static int read_response_block(ne_request *req, struct ne_response *resp, * number of bytes left to read in the current chunk. */ if (resp->body.chunk.remain == 0) { unsigned long chunk_len; + const char *cptr; char *ptr; /* Read chunk-size. */ @@ -956,19 +957,11 @@ static int read_response_block(ne_request *req, struct ne_response *resp, *ptr = '\0'; } - /* Reject things strtoul would otherwise allow */ - ptr = req->respbuf; - if (*ptr == '\0' || *ptr == '-' || *ptr == '+' - || (ptr[0] == '0' && ptr[1] == 'x')) { - return aborted(req, _("Could not parse chunk size"), 0); - } - - /* Limit chunk size to <= UINT_MAX, for sanity; must have - * a following NUL due to chunk-ext handling above. */ - errno = 0; - chunk_len = strtoul(req->respbuf, &ptr, 16); - if (errno || ptr == req->respbuf || (*ptr != '\0' && *ptr != '\r') - || chunk_len == ULONG_MAX || chunk_len > UINT_MAX) { + /* Limit chunk size to <= UINT_MAX, for sanity; must have + * a following NUL due to chunk-ext handling above. */ + chunk_len = ne_strhextoul(req->respbuf, &cptr); + if (errno || (*cptr != '\0' && *cptr != '\r') + || chunk_len > UINT_MAX) { return aborted(req, _("Could not parse chunk size"), 0); } NE_DEBUG(NE_DBG_HTTP, "req: Chunk size: %lu\n", chunk_len); diff --git a/src/ne_string.c b/src/ne_string.c index d423d1b5..625716f4 100644 --- a/src/ne_string.c +++ b/src/ne_string.c @@ -36,6 +36,7 @@ #include #include +#include #include "ne_alloc.h" #include "ne_string.h" @@ -801,3 +802,25 @@ char *ne_strparam(const char *charset, const char *lang, return rv; } + +unsigned long ne_strhextoul(const char *str, const char **end) +{ + unsigned long ret; + char *p; + + if ((str[0] == '0' && (str[1] == 'x' || str[1] == 'X')) + || !((str[0] >= '0' && str[0] <= '9') + || (str[0] >= 'A' && str[0] <= 'Z') + || (str[0] >= 'a' && str[0] <= 'z'))) { + errno = EINVAL; + p = (char *)str; + ret = ULONG_MAX; + } + else { + errno = 0; + ret = strtoul(str, &p, 16); + } + if (end) *end = (const char *)p; + + return ret; +} diff --git a/src/ne_string.h b/src/ne_string.h index d1d0e713..46f54392 100644 --- a/src/ne_string.h +++ b/src/ne_string.h @@ -220,6 +220,13 @@ char *ne_strparam(const char *charset, const char *lang, const unsigned char *value) ne_attribute((nonnull (1, 3))) ne_attribute_malloc; +/* Parse a hex string like strtoul(,,16), but: + * a) any whitespace, 0x or -/+ prefixes result in EINVAL + * b) errno is always set (to zero or an error) + * c) end pointer is const char * + */ +unsigned long ne_strhextoul(const char *str, const char **endptr); + NE_END_DECLS #endif /* NE_STRING_H */ diff --git a/src/neon.vers b/src/neon.vers index 6c5343fc..590972c9 100644 --- a/src/neon.vers +++ b/src/neon.vers @@ -51,3 +51,6 @@ NEON_0_34 { ne_sock_getproto; }; +NEON_0_35 { + ne_strhextoul; +}; diff --git a/test/request.c b/test/request.c index 37222d73..01ee511b 100644 --- a/test/request.c +++ b/test/request.c @@ -1491,6 +1491,8 @@ static int fail_on_invalid(void) "Could not parse chunk size" }, { RESP200 TE_CHUNKED "\r\n" "0x5\r\n" VALID_ABCDE, "Could not parse chunk size" }, + { RESP200 TE_CHUNKED "\r\n" "0X5\r\n" VALID_ABCDE, + "Could not parse chunk size" }, { RESP200 TE_CHUNKED "\r\n" "+5\r\n" VALID_ABCDE, "Could not parse chunk size" }, { RESP200 TE_CHUNKED "\r\n" "5 5\r\n" VALID_ABCDE, diff --git a/test/string-tests.c b/test/string-tests.c index 512fd8aa..4b7e447f 100644 --- a/test/string-tests.c +++ b/test/string-tests.c @@ -29,6 +29,9 @@ #ifdef HAVE_ERRNO_H #include /* for the ENOENT definitions in str_errors */ #endif +#ifdef HAVE_LIMITS_H +#include +#endif #include "ne_string.h" #include "ne_utils.h" @@ -783,6 +786,58 @@ static int strparam(void) return OK; } +static int strhextoul(void) +{ + static const struct { + const char *input; + unsigned long expect; + int error; /* errno */ + const char *endp; + } *t, ts[] = { + { "0x0", ULONG_MAX, EINVAL }, + { "0X1", ULONG_MAX, EINVAL }, + { "+1", ULONG_MAX, EINVAL }, + { "-0", ULONG_MAX, EINVAL }, + { "+0x1", ULONG_MAX, EINVAL }, + { "+0X1", ULONG_MAX, EINVAL }, + { "", ULONG_MAX, EINVAL }, + { "1", 1, 0, "" }, + { " 10", ULONG_MAX, EINVAL, " 10" }, + { "0000010 ", 16, 0, " " }, + { "4242", 16962, 0 }, + { "4242zZZz", 16962, 0, "zZZz" }, + { "cAfEBeEf", 3405692655 }, +#if SIZEOF_LONG == 8 + { "10000000000000000", ULONG_MAX, ERANGE }, + { "100000000", 0x100000000, 0 }, +#elif SIZEOF_LONG == 4 + { "100000000", ULONG_MAX, ERANGE }, +#endif + { NULL } + }; + unsigned n; + + for (n = 0; ts[n].input; n++) { + unsigned long actual; + const char *endp = "(unset)", **endpp; + int errnum; + + t = ts + n; + + endpp = t->endp ? &endp : NULL; + errno = ENOENT; + actual = ne_strhextoul(t->input, endpp); + errnum = errno; + ONV(errnum != t->error, + ("got errno %d not %d for [%s]", errnum, t->error, t->input)); + ONV(actual != t->expect, + ("got %lu not %lu for [%s]", actual, t->expect, t->input)); + if (endpp) ONCMP(*endpp, t->endp); + } + + return OK; +} + ne_test tests[] = { T(simple), T(buf_concat), @@ -816,6 +871,7 @@ ne_test tests[] = { T(strhash_sha_512), T(strhash_sha_512_256), T(strparam), + T(strhextoul), T(NULL) };