From 762224a9631e89002f12ba82799be69f19299d23 Mon Sep 17 00:00:00 2001 From: Ken Murchison Date: Sat, 9 Dec 2023 16:43:41 -0500 Subject: [PATCH] imparse.c: limit range_t to 32-bit integers (per RFC 9051) Also add unit tests --- cunit/imparse.testc | 59 +++++++++++++++++++++++++++++++++++++++++++++ imap/imapd.c | 2 +- imap/index.c | 2 +- imap/search_query.c | 2 +- lib/imparse.c | 16 ++++++++---- lib/imparse.h | 6 ++--- 6 files changed, 76 insertions(+), 11 deletions(-) diff --git a/cunit/imparse.testc b/cunit/imparse.testc index 27ba9877a89..9b56649b818 100644 --- 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 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 index 3bb5c4bb6fb..7a5715f8da4 100644 --- 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 index 17d5204e02a..7db922c8f10 100644 --- 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 index c73d671f998..999d352c0ad 100644 --- 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 index 05aa4cdfdf0..0f2ccd2f4f4 100644 --- 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 index 2e8853f8a7d..bf7ea26ac7e 100644 --- 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);