Re: Null-Dereference bug in vector.c

[Akim Demaille]

Hi!

> Le 24 août 2019 à 09:31, 江 祖铭 <address@hidden> a écrit :
> 
> The maintainers of bison:
> 
> Hello. I am Zu-Ming Jiang, a master student at Tsinghua University
> 
> I find a null dereference bug in vector.c
> 
> Describe the bug：
> the realloc() in the call stack shown below may fail:
> #1 Call realloc() in vbitset_resize(), at vector.c: 77
> #2 Call vbitset_resize() in vbitset_init(), at bitset.c: 989
> #3 Call vbitset_init() in bitset_init(), at bitset.c: 88
> #4 Call bitset_init() in bitset_alloc(), at bitset.c: 138
> #5 Call bitset_alloc() in bitset_create(), at bitset.c: 163
> #6 Call bitset_create() in pack_table(), at table.c: 727
> #7 Call pack_table() in tables_generate(), at table.c: 802
> #8 Call tables_generate() in main(), at main.c: 152
> 
> If the realloc() in this call stack fails, It will make the variable 
> VBITSET_WORDS (src) become NULL. And then, the program executes the 
> instruction memset (VBITSET_WORDS (src) + oldsize, 0,  (newsize - oldsize) * 
> sizeof (bitset_word)). So a crash occur because the program memset a buffer 
> that begin at NULL.

Thanks for the report!  This was addressed in the following commit:

commit 09a4bfdab493beeb637c0a5a44f8f09836e3008d
Author: Akim Demaille <address@hidden>
Date:   Thu Sep 5 11:38:02 2019 +0200

    gnulib: update
    
    Contains a fix for
    https://lists.gnu.org/archive/html/bug-bison/2019-08/msg00016.html.
    See
    https://lists.gnu.org/archive/html/bug-gnulib/2019-09/msg00005.html.
    Reported by 江 祖铭 (Zu-Ming Jiang).

diff --git a/gnulib b/gnulib
index 231b2a88..03add7eb 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 231b2a884fe695a5bc221e545440210c94f44eea
+Subproject commit 03add7eb9d06ab509034ba01c904a4cb36f5706b


FTR, the "real" fix is:

commit d3bb911fd0f53b2c69c96da43cc0f6557f3d2dd8
Author: Akim Demaille <address@hidden>
Date:   Thu Sep 5 11:36:39 2019 +0200

    bitset: check memory allocation
    
    Reported by 江 祖铭 (Zu-Ming Jiang).
    With help from Paul Eggert.
    https://lists.gnu.org/archive/html/bug-bison/2019-08/msg00016.html
    
    * lib/bitset/table.c (tbitset_resize): When growing, use xrealloc
    instead of realloc.
    When shrinking, accept failures.
    * lib/bitset/vector.c (vbitset_resize): Likewise.

diff --git a/ChangeLog b/ChangeLog
index 28a6b264b..077471de9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2019-09-06  Akim Demaille  <address@hidden>
+
+       bitset: check memory allocation
+       Reported by 江 祖铭 (Zu-Ming Jiang).
+       With help from Paul Eggert.
+       https://lists.gnu.org/archive/html/bug-bison/2019-08/msg00016.html
+       * lib/bitset/table.c (tbitset_resize): When growing, use xrealloc
+       instead of realloc.
+       When shrinking, accept failures.
+       * lib/bitset/vector.c (vbitset_resize): Likewise.
+
 2019-09-07  Paul Eggert  <address@hidden>
 
        scratch_buffer: sync from glibc
diff --git a/lib/bitset/table.c b/lib/bitset/table.c
index 07184d657..7691d9805 100644
--- a/lib/bitset/table.c
+++ b/lib/bitset/table.c
@@ -25,6 +25,7 @@
 #include <string.h>
 
 #include "obstack.h"
+#include "xalloc.h"
 
 /* This file implements expandable bitsets.  These bitsets can be of
    arbitrary length and are more efficient than arrays of bits for
@@ -142,7 +143,7 @@ tbitset_resize (bitset src, bitset_bindex n_bits)
 
           bitset_windex size = oldsize == 0 ? newsize : newsize + newsize / 4;
           EBITSET_ELTS (src)
-            = realloc (EBITSET_ELTS (src), size * sizeof (tbitset_elt *));
+            = xrealloc (EBITSET_ELTS (src), size * sizeof (tbitset_elt *));
           EBITSET_ASIZE (src) = size;
         }
 
@@ -155,9 +156,13 @@ tbitset_resize (bitset src, bitset_bindex n_bits)
          the memory unless it is shrinking by a reasonable amount.  */
       if ((oldsize - newsize) >= oldsize / 2)
         {
-          EBITSET_ELTS (src)
+          void *p
             = realloc (EBITSET_ELTS (src), newsize * sizeof (tbitset_elt *));
-          EBITSET_ASIZE (src) = newsize;
+          if (p)
+            {
+              EBITSET_ELTS (src) = p;
+              EBITSET_ASIZE (src) = newsize;
+            }
         }
 
       /* Need to prune any excess bits.  FIXME.  */
diff --git a/lib/bitset/vector.c b/lib/bitset/vector.c
index 54f148d56..5e543283a 100644
--- a/lib/bitset/vector.c
+++ b/lib/bitset/vector.c
@@ -24,6 +24,8 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "xalloc.h"
+
 /* This file implements variable size bitsets stored as a variable
    length array of words.  Any unused bits in the last word must be
    zero.
@@ -74,7 +76,7 @@ vbitset_resize (bitset src, bitset_bindex n_bits)
 
           bitset_windex size = oldsize == 0 ? newsize : newsize + newsize / 4;
           VBITSET_WORDS (src)
-            = realloc (VBITSET_WORDS (src), size * sizeof (bitset_word));
+            = xrealloc (VBITSET_WORDS (src), size * sizeof (bitset_word));
           VBITSET_ASIZE (src) = size;
         }
 
@@ -88,9 +90,13 @@ vbitset_resize (bitset src, bitset_bindex n_bits)
          the memory unless it is shrinking by a reasonable amount.  */
       if ((oldsize - newsize) >= oldsize / 2)
         {
-          VBITSET_WORDS (src)
+          void *p
             = realloc (VBITSET_WORDS (src), newsize * sizeof (bitset_word));
-          VBITSET_ASIZE (src) = newsize;
+          if (p)
+            {
+              VBITSET_WORDS (src) = p;
+              VBITSET_ASIZE (src) = newsize;
+            }
         }
 
       /* Need to prune any excess bits.  FIXME.  */