Fix arithmetic overflow issues in sexp parser.

diff --git a/ChangeLog b/ChangeLog
index 66dea15a1cc4ea15297706f8cc8d1eef02118870..470df6fb903ea344163aab2e3e8aa5ac0dafd2f0 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2026-04-30  Niels Möller  <nisse@lysator.liu.se>
+
+       * sexp.c: Fix bugs reported by Sebastián Alba Vives.
+       (sexp_iterator_init): Fix truncation bug, by using size_t
+       for the length argument.
+       (sexp_iterator_simple): Check for arithmetic overflow when parsing
+       the decimal length field, and compare length to available input
+       only once.
+       * testsuite/sexp-test.c (test_main): Add tests for truncated
+       strings, including a test triggering 64-bit arithmetic overflow.
+
 2026-04-03  Niels Möller  <nisse@lysator.liu.se>
 
        * drbg-ctr-aes256.c (drbg_ctr_aes256_init): Use const for

diff --git a/sexp.c b/sexp.c
index fe09a1a4c3428530974881443a21c4e516e0e158..f4f24ebf9d8cd4e1c97f9f190cc7a4fdfe5a6a22 100644 (file)
--- a/sexp.c
+++ b/sexp.c
@@ -47,7 +47,7 @@
  * first element. */
 static void
 sexp_iterator_init(struct sexp_iterator *iterator,
-                  unsigned length, const uint8_t *input)
+                  size_t length, const uint8_t *input)
 {
   iterator->length = length;
   iterator->buffer = input;
@@ -68,7 +68,7 @@ sexp_iterator_simple(struct sexp_iterator *iterator,
                     size_t *size,
                     const uint8_t **string)
 {
-  unsigned length = 0;
+  size_t length = 0;
   uint8_t c;
   
   if (EMPTY(iterator)) return 0;
@@ -78,9 +78,14 @@ sexp_iterator_simple(struct sexp_iterator *iterator,
   if (c >= '1' && c <= '9')
     do
       {
-       length = length * 10 + (c - '0');
-       /* >= to account for ':' character */
-       if (length >= (iterator->length - iterator->pos))
+       unsigned digit = c - '0';
+       if (length > (~(size_t) 0) / 10)
+         /* Multiply by 10 would overflow. */
+         return 0;
+
+       length = length * 10 + digit;
+       if (length < digit)
+         /* Overflow from addition. */
          return 0;
 
        if (EMPTY(iterator)) return 0;
@@ -97,6 +102,9 @@ sexp_iterator_simple(struct sexp_iterator *iterator,
   if (c != ':')
     return 0;
 
+  if (length > (iterator->length - iterator->pos))
+    return 0;
+
   *size = length;
   *string = iterator->buffer + iterator->pos;
   iterator->pos += length;

diff --git a/testsuite/sexp-test.c b/testsuite/sexp-test.c
index 84b228cdd7e6f0c66cbb5d21b24b587638af6ebc..41a912e476863e8e408c7d68e59c8f8085afb835 100644 (file)
--- a/testsuite/sexp-test.c
+++ b/testsuite/sexp-test.c
@@ -33,6 +33,20 @@ test_main(void)
   ASSERT(sexp_iterator_get_uint32(&i, &x) && x == 0x80);
   ASSERT(sexp_iterator_get_uint32(&i, &x) && x == 0xaabbccdd);
 
+  /* Valid and truncated items. */
+  ASSERT(sexp_iterator_first(&i, LDATA("3:foo")));
+  ASSERT(i.type == SEXP_ATOM
+        && !i.display_length && !i.display
+        && i.atom_length == 3 && MEMEQ(3, "foo", i.atom)
+        && sexp_iterator_next(&i) && i.type == SEXP_END);
+
+  /* Truncated. */
+  ASSERT(!sexp_iterator_first(&i, LDATA("3")));
+  ASSERT(!sexp_iterator_first(&i, LDATA("3:")));
+  ASSERT(!sexp_iterator_first(&i, LDATA("4:foo")));
+  /* Potentially overflowing, length = 2^64. */
+  ASSERT(!sexp_iterator_first(&i, LDATA("18446744073709551616:foo")));
+
   ASSERT(sexp_iterator_first(&i, LDATA("3:foo0:[3:bar]12:xxxxxxxxxxxx")));
   ASSERT(i.type == SEXP_ATOM
         && !i.display_length && !i.display