CVE-2020-24240: Use After Free (CWE-416)
value: GNU Bison before 3.7.1 has a use-after-free in _obstack_free in lib/obstack.c (called from gram_lex) when a '\0' byte is encountered. NOTE: there is a risk only if Bison is used with untrusted input, and the observed bug happens to cause unsafe behavior with a specific compiler/architecture. The bug report was intended to show that a crash may occur in Bison itself, not that a crash may occur in code that is generated by Bison.
CWE description: The product reuses or references memory after it has been freed. At some point afterward, the memory may be allocated again and saved in another pointer, while the original pointer references a location somewhere within the new allocation. Any operations using the original pointer are no longer valid because the memory "belongs" to the code that operates on the new pointer.
CVSS v3 base score: 5.5
Published: 2020-08-25T14:15Z
Repository: https://github.com/akimd/bison
Language: Other
Files changed: THANKS, src/flex-scanner.h, src/scan-gram.l, tests/input.at
Commit date: 2020-07-28 19:01:48 +0200
Commit message: scanner: don't crash on strings containing a NUL byte

We crash if the input contains a string containing a NUL byte.
Reported by Suhwan Song.
https://lists.gnu.org/r/bug-bison/2020-07/msg00051.html

* src/flex-scanner.h (STRING_FREE): Avoid accidental use of
last_string.
* src/scan-gram.l: Don't call STRING_FREE without calling
STRING_FINISH first.
* tests/input.at (Invalid inputs): Check that case.

Diff: diff --git a/THANKS b/THANKS
index ac073ea6..5c64da3c 100644
--- a/THANKS
+++ b/THANKS
@@ -183,10 +183,11 @@ Sergei Steshenko          sergstesh@yahoo.com
 Shura                     debil_urod@ngs.ru
 Simon Sobisch             simonsobisch@web.de
 Stefano Lattarini         stefano.lattarini@gmail.com
 Stephen Cameron           stephenmcameron@gmail.com
 Steve Murphy              murf@parsetree.com
+Suhwan Song               prada960808@gmail.com
 Sum Wu                    sum@geekhouse.org
 Théophile Ranquet         theophile.ranquet@gmail.com
 Thiru Ramakrishnan        thiru.ramakrishnan@gmail.com
 Thomas Jahns              jahns@dkrz.de
 Thomas Petazzoni          thomas.petazzoni@bootlin.com
diff --git a/src/flex-scanner.h b/src/flex-scanner.h
index 56ca7ce3..028847fd 100644
--- a/src/flex-scanner.h
+++ b/src/flex-scanner.h
@@ -110,9 +110,17 @@ static struct obstack obstack_for_string;
   (last_string = obstack_finish0 (&obstack_for_string))
 
 # define STRING_1GROW(Char)                     \
   obstack_1grow (&obstack_for_string, Char)
 
-# define STRING_FREE()                                  \
+# ifdef NDEBUG
+#  define STRING_FREE()                                 \
   obstack_free (&obstack_for_string, last_string)
+# else
+#  define STRING_FREE()                                  \
+  do {                                                   \
+    obstack_free (&obstack_for_string, last_string);     \
+    last_string = NULL;                                  \
+  } while (0)
+#endif
 
 #endif
diff --git a/src/scan-gram.l b/src/scan-gram.l
index f8d85f23..ad2904ce 100644
--- a/src/scan-gram.l
+++ b/src/scan-gram.l
@@ -401,10 +401,11 @@ eqopt    ({sp}=)?
 
 <SC_ESCAPED_CHARACTER,SC_ESCAPED_STRING,SC_ESCAPED_TSTRING,SC_TAG>
 {
   \0         {
     complain (loc, complaint, _("invalid null character"));
+    STRING_FINISH ();
     STRING_FREE ();
     return GRAM_error;
   }
 }
 
@@ -597,11 +598,10 @@ eqopt    ({sp}=)?
 {
   "'" {
     STRING_FINISH ();
     BEGIN INITIAL;
     loc->start = token_start;
-    val->CHAR = last_string[0];
 
     if (last_string[0] == '\0')
       {
         complain (loc, complaint, _("empty character literal"));
         STRING_FREE ();
@@ -613,10 +613,11 @@ eqopt    ({sp}=)?
         STRING_FREE ();
         return GRAM_error;
       }
     else
       {
+        val->CHAR = last_string[0];
         STRING_FREE ();
         return CHAR;
       }
   }
   {eol}     unexpected_newline (token_start, "'");
diff --git a/tests/input.at b/tests/input.at
index 4da63795..effcd1cc 100644
--- a/tests/input.at
+++ b/tests/input.at
@@ -1,6 +1,6 @@
-# Checking the Bison scanner.                    -*- Autotest -*-
+# Checking the Bison reader.                    -*- Autotest -*-
 
 # Copyright (C) 2002-2015, 2018-2020 Free Software Foundation, Inc.
 
 # This program is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -76,34 +76,62 @@ AT_CLEANUP
 
 ## ---------------- ##
 ## Invalid inputs.  ##
 ## ---------------- ##
 
+# The truly bad guys no human would write, but easily uncovered by
+# fuzzers.
 AT_SETUP([Invalid inputs])
 
 AT_DATA([input.y],
 [[\000\001\002\377?
+"\000"
 %%
 ?
 default: 'a' }
 %&
 %a-does-not-exist
 %-
 %{
 ]])
 AT_PERL_REQUIRE([[-pi -e 's/\\(\d{3})/chr(oct($1))/ge' input.y]])
 
-AT_BISON_CHECK([input.y], [1], [],
+AT_BISON_CHECK([-fcaret input.y], [1], [], [stderr])
+
+# Autotest's diffing, when there are NUL bytes, just reports "binary
+# files differ".  So don't leave NUL bytes.
+AT_PERL_CHECK([[-p -e 's{([\0\377])}{sprintf "\\x%02x", ord($1)}ge' stderr]], [],
 [[input.y:1.1-2: error: invalid characters: '\0\001\002\377?'
-input.y:3.1: error: invalid character: '?'
-input.y:4.14: error: invalid character: '}'
-input.y:5.1: error: invalid character: '%'
-input.y:5.2: error: invalid character: '&'
-input.y:6.1-17: error: invalid directive: '%a-does-not-exist'
-input.y:7.1: error: invalid character: '%'
-input.y:7.2: error: invalid character: '-'
-input.y:8.1-9.0: error: missing '%}' at end of file
+    1 | \x00\xff?
+      | ^~
+input.y:2.2: error: invalid null character
+    2 | "\x00"
+      |  ^
+input.y:4.1: error: invalid character: '?'
+    4 | ?
+      | ^
+input.y:5.14: error: invalid character: '}'
+    5 | default: 'a' }
+      |              ^
+input.y:6.1: error: invalid character: '%'
+    6 | %&
+      | ^
+input.y:6.2: error: invalid character: '&'
+    6 | %&
+      |  ^
+input.y:7.1-17: error: invalid directive: '%a-does-not-exist'
+    7 | %a-does-not-exist
+      | ^~~~~~~~~~~~~~~~~
+input.y:8.1: error: invalid character: '%'
+    8 | %-
+      | ^
+input.y:8.2: error: invalid character: '-'
+    8 | %-
+      |  ^
+input.y:9.1-10.0: error: missing '%}' at end of file
+    9 | %{
+      | ^~
 ]])
 
 AT_CLEANUP
 
 

Vulnerable code: # define STRING_FREE()                                  \
    val->CHAR = last_string[0];
# Checking the Bison scanner.                    -*- Autotest -*-
AT_BISON_CHECK([input.y], [1], [],
input.y:3.1: error: invalid character: '?'
input.y:4.14: error: invalid character: '}'
input.y:5.1: error: invalid character: '%'
input.y:5.2: error: invalid character: '&'
input.y:6.1-17: error: invalid directive: '%a-does-not-exist'
input.y:7.1: error: invalid character: '%'
input.y:7.2: error: invalid character: '-'
input.y:8.1-9.0: error: missing '%}' at end of file
Fixed code: Suhwan Song               prada960808@gmail.com
# ifdef NDEBUG
#  define STRING_FREE()                                 \
# else
#  define STRING_FREE()                                  \
  do {                                                   \
    obstack_free (&obstack_for_string, last_string);     \
    last_string = NULL;                                  \
  } while (0)
#endif
    STRING_FINISH ();
        val->CHAR = last_string[0];
# Checking the Bison reader.                    -*- Autotest -*-
# The truly bad guys no human would write, but easily uncovered by
# fuzzers.
"\000"
AT_BISON_CHECK([-fcaret input.y], [1], [], [stderr])

# Autotest's diffing, when there are NUL bytes, just reports "binary
# files differ".  So don't leave NUL bytes.
AT_PERL_CHECK([[-p -e 's{([\0\377])}{sprintf "\\x%02x", ord($1)}ge' stderr]], [],
    1 | \x00\xff?
      | ^~
input.y:2.2: error: invalid null character
    2 | "\x00"
      |  ^
input.y:4.1: error: invalid character: '?'
    4 | ?
      | ^
input.y:5.14: error: invalid character: '}'
    5 | default: 'a' }
      |              ^
input.y:6.1: error: invalid character: '%'
    6 | %&
      | ^
input.y:6.2: error: invalid character: '&'
    6 | %&
      |  ^
input.y:7.1-17: error: invalid directive: '%a-does-not-exist'
    7 | %a-does-not-exist
      | ^~~~~~~~~~~~~~~~~
input.y:8.1: error: invalid character: '%'
    8 | %-
      | ^
input.y:8.2: error: invalid character: '-'
    8 | %-
      |  ^
input.y:9.1-10.0: error: missing '%}' at end of file
    9 | %{
      | ^~