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.
notroj · Dec 22, 2024 · 1e7c960 · 1e7c960
1 parent 9a23c93
commit 1e7c960
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 16 deletions.
diff --git 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
@@ -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
@@ -36,6 +36,7 @@
 
 #include <stdio.h>
 #include <assert.h>
+#include <errno.h>
 
 #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
@@ -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
@@ -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
@@ -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
@@ -29,6 +29,9 @@
 #ifdef HAVE_ERRNO_H
 #include <errno.h> /* for the ENOENT definitions in str_errors */
 #endif
+#ifdef HAVE_LIMITS_H
+#include <limits.h>
+#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)
 };