obstack: fix unlikely buffer overrun in glibc

This bug is not plausible on any Gnulib platform,
but it could happen in glibc if the requested size is UINT_MAX.
* lib/obstack.c (_obstack_newchunk): Fail if requested size is 0.
* lib/obstack.in.h (obstack_grow0): Check for room <= len,
not room <= len + 1 where len + 1 could overflow.

diff --git a/ChangeLog b/ChangeLog
index adf06a2645f0205b4ac3265281000111358ac2ca..9ee3f6eea5e0858a4e49c0baa435ea799470c61c 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2025-05-05  Paul Eggert  <eggert@cs.ucla.edu>
+
+       obstack: fix unlikely buffer overrun in glibc
+       This bug is not plausible on any Gnulib platform,
+       but it could happen in glibc if the requested size is UINT_MAX.
+       * lib/obstack.c (_obstack_newchunk): Fail if requested size is 0.
+       * lib/obstack.in.h (obstack_grow0): Check for room <= len,
+       not room <= len + 1 where len + 1 could overflow.
+
 2025-05-05  Bruno Haible  <bruno@clisp.org>
 
        windows-cygpath: Make it work with the native Windows 'git' port.

diff --git a/lib/obstack.c b/lib/obstack.c
index 0f6c5f98d7ad13fd9368e93e69ffd5686c8ae43f..5839e04f2f270c1029f7138ffe1a7640eed0f77a 100644 (file)
--- a/lib/obstack.c
+++ b/lib/obstack.c
@@ -34,8 +34,19 @@
 # include <alignof.h>
 # define __alignof__(type) alignof_type (type)
 #endif
-#include <stdlib.h>
+
+#include <limits.h>
 #include <stdint.h>
+#include <stdlib.h>
+
+/* Some work would need to be done to port this module
+   to unusual platforms where size_t fits in int.
+   For now, document the assumption that INT_MAX < SIZE_MAX,
+   and therefore size_t calculations are modulo SIZE_MAX + 1
+   instead of having undefined behavior on overflow.  */
+#if SIZE_MAX <= INT_MAX
+ #error "SIZE_MAX <= INT_MAX"
+#endif
 
 #ifndef MAX
 # define MAX(a,b) ((a) > (b) ? (a) : (b))
@@ -154,6 +165,7 @@ _obstack_begin_1 (struct obstack *h,
 /* Allocate a new current chunk for the obstack *H
    on the assumption that LENGTH bytes need to be added
    to the current object, or a new object of length LENGTH allocated.
+   Fail if LENGTH <= 0, as this means obstack_grow0's length overflowed.
    Copies any partial object from the end of the old chunk
    to the beginning of the new one.  */
 
@@ -174,8 +186,9 @@ _obstack_newchunk (struct obstack *h, _OBSTACK_SIZE_T length)
   if (new_size < h->chunk_size)
     new_size = h->chunk_size;
 
-  /* Allocate and initialize the new chunk.  */
-  if (obj_size <= sum1 && sum1 <= sum2)
+  /* Allocate and initialize the new chunk,
+     checking for overflow and for nonpositive LENGTH.  */
+  if (obj_size < sum1 && sum1 <= sum2)
     new_chunk = call_chunkfun (h, new_size);
   if (!new_chunk)
     (*obstack_alloc_failed_handler)();

diff --git a/lib/obstack.in.h b/lib/obstack.in.h
index 5296cf5820d8cb8fb341a343e14ec7ed9939c3da..1d6ff923b48317c8ae6bdad3f40e1b8b46faa5ea 100644 (file)
--- a/lib/obstack.in.h
+++ b/lib/obstack.in.h
@@ -348,7 +348,7 @@ extern int obstack_exit_failure;
   __extension__                                                                      \
     ({ struct obstack *__o = (OBSTACK);                                              \
        _OBSTACK_SIZE_T __len = (length);                                     \
-       if (obstack_room (__o) < __len + 1)                                   \
+       if (obstack_room (__o) <= __len)                                              \
          _obstack_newchunk (__o, __len + 1);                                 \
        memcpy (__o->next_free, where, __len);                                \
        __o->next_free += __len;                                                      \
@@ -484,7 +484,7 @@ extern int obstack_exit_failure;
 
 # define obstack_grow0(h, where, length)                                     \
   ((h)->temp.i = (length),                                                   \
-   ((obstack_room (h) < (h)->temp.i + 1)                                     \
+   ((obstack_room (h) <= (h)->temp.i)                                        \
    ? (_obstack_newchunk ((h), (h)->temp.i + 1), 0) : 0),                     \
    memcpy ((h)->next_free, where, (h)->temp.i),                                      \
    (h)->next_free += (h)->temp.i,                                            \