imparse.c: limit range_t to 32-bit integers (per RFC 9051)

Also add unit tests
cyrusimap · Dec 9, 2023 · 762224a · 762224a
1 parent 640192c
commit 762224a
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 11 deletions.
diff --git a/cunit/imparse.testc b/cunit/imparse.testc
@@ -62,3 +62,62 @@ static void test_isatom(void)
 /* XXX test imparse_issequence() */
 
 /* XXX test imparse_isnumber() */
+
+static void test_parsge_range(void)
+{
+    range_t range;
+
+    /*
+     * https://tools.ietf.org/html/rfc9051#name-formal-syntax
+     *
+     * nz-number       = digit-nz *DIGIT
+     *                     ; Non-zero unsigned 32-bit integer
+     *                     ; (0 < n < 4,294,967,296)
+     *
+     *
+     * https://tools.ietf.org/html/rfc9394#name-formal-syntax
+     *
+     * MINUS               = "-"
+     *
+     * partial-range-first = nz-number ":" nz-number
+     *     ;; Request to search from oldest (lowest UIDs) to
+     *     ;; more recent messages.
+     *     ;; A range 500:400 is the same as 400:500.
+     *     ;; This is similar to <seq-range> from [RFC3501]
+     *     ;; but cannot contain "*".
+     *
+     * partial-range-last  = MINUS nz-number ":" MINUS nz-number
+     *     ;; Request to search from newest (highest UIDs) to
+     *     ;; oldest messages.
+     *     ;; A range -500:-400 is the same as -400:-500.
+     *
+     * partial-range       = partial-range-first / partial-range-last
+     */
+
+    CU_ASSERT_EQUAL(imparse_range("1:1", &range), 0);
+    CU_ASSERT_EQUAL(imparse_range("1:2", &range), 0);
+    CU_ASSERT_EQUAL(imparse_range("2:1", &range), 0);
+    CU_ASSERT_EQUAL(imparse_range("-1:-2", &range), 0);
+    CU_ASSERT_EQUAL(imparse_range("-2:-1", &range), 0);
+
+    CU_ASSERT_EQUAL(imparse_range("1:-2", &range), 0);
+    CU_ASSERT_EQUAL(imparse_range("-1:2", &range), 0);
+
+    CU_ASSERT_NOT_EQUAL(imparse_range("0:1", &range), 0);
+    CU_ASSERT_NOT_EQUAL(imparse_range("--1:-2", &range), 0);
+    CU_ASSERT_NOT_EQUAL(imparse_range("+1:-2", &range), 0);
+
+    CU_ASSERT_NOT_EQUAL(imparse_range("1", &range), 0);
+    CU_ASSERT_NOT_EQUAL(imparse_range("1:", &range), 0);
+    CU_ASSERT_NOT_EQUAL(imparse_range(":1", &range), 0);
+
+    CU_ASSERT_NOT_EQUAL(imparse_range("1:a", &range), 0);
+    CU_ASSERT_NOT_EQUAL(imparse_range("-1:-a", &range), 0);
+    CU_ASSERT_NOT_EQUAL(imparse_range("1a:2", &range), 0);
+    CU_ASSERT_NOT_EQUAL(imparse_range("1:2a", &range), 0);
+
+    CU_ASSERT_NOT_EQUAL(imparse_range("1:4294967296", &range), 0);
+    CU_ASSERT_NOT_EQUAL(imparse_range("-1:-4294967296", &range), 0);
+    CU_ASSERT_NOT_EQUAL(imparse_range("1:18446744073709551616", &range), 0);
+    CU_ASSERT_NOT_EQUAL(imparse_range("-1:-18446744073709551616", &range), 0);
+}
diff --git a/imap/imapd.c b/imap/imapd.c
@@ -5002,7 +5002,7 @@ static int parse_fetch_args(const char *tag, const char *cmd,
     struct octetinfo oi;
     strarray_t *newfields = strarray_new();
 
-    fa->partial.high = ULONG_MAX;
+    fa->partial.high = UINT32_MAX;
 
     c = getword(imapd_in, &fetchatt);
     if (c == '(' && !fetchatt.s[0]) {

diff --git a/imap/index.c b/imap/index.c
@@ -2199,7 +2199,7 @@ EXPORTED int index_search(struct index_state *state,
         if (searchargs->returnopts & SEARCH_RETURN_PARTIAL) {
             const char *sign = searchargs->partial.is_last ? "-" : "";
 
-            prot_printf(state->out, " PARTIAL (%s%lu:%s%lu %s)",
+            prot_printf(state->out, " PARTIAL (%s%u:%s%u %s)",
                         sign, searchargs->partial.low,
                         sign, searchargs->partial.high,
                         seqstr ? seqstr : "NIL");

diff --git a/imap/search_query.c b/imap/search_query.c
@@ -558,7 +558,7 @@ static int _subquery_run_one_folder(search_query_t *query,
         break;
 
     default:
-        if (!searchargs->partial.high) searchargs->partial.high = ULONG_MAX;
+        if (!searchargs->partial.high) searchargs->partial.high = UINT32_MAX;
         break;
     }
 

diff --git a/lib/imparse.c b/lib/imparse.c
@@ -227,20 +227,26 @@ EXPORTED int imparse_range(char *s, range_t *range)
         range->is_last = 1;
         s++;
     }
-    else if (!Uisdigit(*s)) return -1;
+    if (!Uisdigit(*s)) return -1;
 
     errno = 0;
     range->low = strtoul(s, &s, 10);
-    if (!range->low || errno || *s != ':') return -1;
+    if (!range->low || range->low > UINT32_MAX || errno || *s != ':') {
+        errno = 0;
+        return -1;
+    }
 
     if (*++s == '-') {
         if (!range->is_last) return -1;
         s++;
     }
-    else if (!Uisdigit(*s)) return -1;
+    if (!Uisdigit(*s)) return -1;
 
-    range->high = strtoul(s, NULL, 10);
-    if (!range->high || errno) return -1;
+    range->high = strtoul(s, &s, 10);
+    if (!range->high || range->low > UINT32_MAX  || errno || *s) {
+        errno = 0;
+        return -1;
+    }
 
     if (range->low > range->high) {
         unsigned long n = range->high;

diff --git a/lib/imparse.h b/lib/imparse.h
@@ -44,9 +44,9 @@
 #define INCLUDED_IMPARSE_H
 
 typedef struct {
-    unsigned long low;
-    unsigned long high;
-    unsigned is_last : 1;
+    uint32_t low;
+    uint32_t high;
+    u_char   is_last : 1;
 } range_t;
 
 extern int imparse_word (char **s, char **retval);
-Original file line number
+Diff line change
@@ Expand Up / @@ -558,7 +558,7 @@ static int _subquery_run_one_folder(search_query_t *query, @@
             break;
         default:
-            if (!searchargs->partial.high) searchargs->partial.high = ULONG_MAX;
+            if (!searchargs->partial.high) searchargs->partial.high = UINT32_MAX;
             break;
         }
@@ Expand Down @@