Re: [bison 3.7] Bug Report

Hi Suhwan,

> Le 28 juil. 2020 à 11:59, Suhwan Song <[email protected]> a écrit :
> 
> Hi, I'm Suhwan Song from Seoul National University.
> 
> I found the use-after-free bug in bison 3.7 which is the latest version.
> I attached the poc file and the asan log.

Thanks for the bug report.  I will installed the patch below.  It will ship with Bison 3.7.1.

Cheers!



commit bfc38ef10501e359fe3df8e4ab8e9441af2cb267
Author: Akim Demaille <[email protected]>
Date:   Tue Jul 28 18:51:30 2020 +0200

    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 --git a/THANKS b/THANKS
index ac073ea6..5c64da3c 100644
--- a/THANKS
+++ b/THANKS
@@ -185,6 +185,7 @@ Simon Sobisch             [email protected]
 Stefano Lattarini         [email protected]
 Stephen Cameron           [email protected]
 Steve Murphy              [email protected]
+Suhwan Song               [email protected]
 Sum Wu                    [email protected]
 Théophile Ranquet         [email protected]
 Thiru Ramakrishnan        [email protected]
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
@@ -112,7 +112,15 @@ static struct obstack 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
@@ -403,6 +403,7 @@ eqopt    ({sp}=)?
 {
   \0         {
     complain (loc, complaint, _("invalid null character"));
+    STRING_FINISH ();
     STRING_FREE ();
     return GRAM_error;
   }
@@ -599,7 +600,6 @@ eqopt    ({sp}=)?
     STRING_FINISH ();
     BEGIN INITIAL;
     loc->start = token_start;
-    val->CHAR = last_string[0];
 
     if (last_string[0] == '\0')
       {
@@ -615,6 +615,7 @@ eqopt    ({sp}=)?
       }
     else
       {
+        val->CHAR = last_string[0];
         STRING_FREE ();
         return CHAR;
       }
diff --git a/tests/input.at b/tests/input.at
index 4da63795..effcd1cc 100644
--- a/tests/input.at
+++ b/tests/input.at
@@ -1,4 +1,4 @@
-# Checking the Bison scanner.                    -*- Autotest -*-
+# Checking the Bison reader.                    -*- Autotest -*-
 
 # Copyright (C) 2002-2015, 2018-2020 Free Software Foundation, Inc.
 
@@ -78,10 +78,13 @@ 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' }
@@ -92,16 +95,41 @@ default: 'a' }
 ]])
 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