[PATCH 10/13] gdb: fix out-of-bounds reads in lex_char_literal

[Simon Marchi <simon.marchi@efficios.com>]

From: Simon Marchi <simon.marchi@polymtl.ca>

The C++ name parser reads out of bounds when the input is

  C<'

lex_char_literal reads the terminating null character as the character of
the literal and advances past it.  It then reads one more character to
check for the closing quote, which is past the end of the string.  Add a
'\0' check to handle that.

Similar problem when the input ends right after a backslash, as in

  C<'\

cp_parse_escape means to handle this case in the switch: it rewinds
*string_ptr to leave it pointing to the nul character and returns 0.
However, we don't even reach that point, because c_parse_backslash returns
true and cp_parse_escape returns early.

c_parse_backslash returns true because it looks up the nul character in
backslashable, which returns a match.  Once we return to lex_char_literal,
the string pointer points past the end of the string, and the quote check
does an out of bounds read.

Fix it by:

 - making cp_parse_escape check for the null character before calling
   c_parse_backslash, so that it handles it as intended

 - reporting an "unterminated character constant" error when the
   character following the opening quote is the null character

 - reporting an "unterminated character constant" error when the input
   ends where the closing quote is expected.  This also handles inputs
   like:

     C<'a

Add an assert in c_parse_backslash and modernize it a bit.

The comment of cp_parse_escape says that, when the backslash is followed by
a null character, it returns a negative value and leaves the string pointer
on the null character.  The negative value part is not true, change it to
say that it returns 0.  The "leaves the string pointer on the null
character" part becomes true with this patch.

Add selftests for these cases.

Change-Id: I94e81e6bc01841ed4f247b19c5ed91a8d8e45521
---
 gdb/cp-name-parser.c | 47 ++++++++++++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/gdb/cp-name-parser.c b/gdb/cp-name-parser.c
index 931f74e8ed51..38ab258a3cfc 100644
--- a/gdb/cp-name-parser.c
+++ b/gdb/cp-name-parser.c
@@ -452,16 +452,20 @@ static const char backslashable[] = "abefnrtv";
 static const char represented[] = "\a\b\e\f\n\r\t\v";
 
 /* Translate the backslash the way we would in the host character set.  */
+
 static int
 c_parse_backslash (int host_char, int *target_char)
 {
-  const char *ix;
-  ix = strchr (backslashable, host_char);
-  if (! ix)
-    return 0;
-  else
-    *target_char = represented[ix - backslashable];
-  return 1;
+  gdb_assert (host_char != '\0');
+
+  if (const char *ix = strchr (backslashable, host_char);
+      ix != nullptr)
+    {
+      *target_char = represented[ix - backslashable];
+      return 1;
+    }
+
+  return 0;
 }
 
 /* Parse a C escape sequence.  STRING_PTR points to a variable
@@ -470,8 +474,8 @@ c_parse_backslash (int host_char, int *target_char)
    is updated past the characters we use.  The value of the
    escape sequence is returned.
 
-   If \ is followed by a null character, we return a negative
-   value and leave the string pointer pointing at the null character.
+   If \ is followed by a null character, we return 0 and leave the
+   string pointer pointing at the null character.
 
    If \ is followed by 000, we return 0 and leave the string pointer
    after the zeros.  A value of 0 does not mean end of string.  */
@@ -481,14 +485,18 @@ cp_parse_escape (const char **string_ptr)
 {
   int target_char;
   int c = *(*string_ptr)++;
+
+  if (c == '\0')
+    {
+      (*string_ptr)--;
+      return 0;
+    }
+
   if (c_parse_backslash (c, &target_char))
     return target_char;
   else
     switch (c)
       {
-      case 0:
-	(*string_ptr)--;
-	return 0;
       case '0':
       case '1':
       case '2':
@@ -550,13 +558,23 @@ lex_char_literal (cp_name_parser_YYSTYPE *lvalp,
       cpname_yyerror (state, _("empty character constant"));
       return ERROR;
     }
+  else if (c == '\0')
+    {
+      cpname_yyerror (state, _("unterminated character constant"));
+      return ERROR;
+    }
 
   /* We over-allocate here, but it doesn't really matter.  */
   char *copy = (char *) obstack_alloc (&state->demangle_info->obstack, 30);
   xsnprintf (copy, 30, "%d", c);
 
   c = *state->lexptr++;
-  if (c != '\'')
+  if (c == '\0')
+    {
+      cpname_yyerror (state, _("unterminated character constant"));
+      return ERROR;
+    }
+  else if (c != '\'')
     {
       cpname_yyerror (state, _("invalid character constant"));
       return ERROR;
@@ -1086,6 +1104,9 @@ parser_tests ()
 
   should_not_parse ("C<''>", "empty character constant");
   should_not_parse ("C<'ab'>", "invalid character constant");
+  should_not_parse ("C<'", "unterminated character constant");
+  should_not_parse ("C<'a", "unterminated character constant");
+  should_not_parse ("C<'\\", "unterminated character constant");
 }
 
 #endif
-- 
2.56.0