diff mbox

libiberty: demangler crash with missing :? or fold expression component.

Message ID 1479166798-9977-1-git-send-email-mark@klomp.org
State New
Headers show

Commit Message

Mark Wielaard Nov. 14, 2016, 11:39 p.m. UTC
When construction a :? or fold expression that requires a third
expression only the first and second were explicitly checked to
not be NULL. Since the third expression is also required in these
constructs it needs to be explicitly checked and rejected when missing.
Otherwise the demangler will crash once it tries to d_print the
NULL component. Added two examples to demangle-expected of strings
that would crash before this fix.

Found by American Fuzzy Lop (afl) fuzzer.
---
 libiberty/ChangeLog                   | 7 +++++++
 libiberty/cp-demangle.c               | 4 ++++
 libiberty/testsuite/demangle-expected | 8 ++++++++
 3 files changed, 19 insertions(+)

Comments

Ian Lance Taylor Nov. 15, 2016, 3:29 p.m. UTC | #1
On Mon, Nov 14, 2016 at 3:39 PM, Mark Wielaard <mark@klomp.org> wrote:
> When construction a :? or fold expression that requires a third
> expression only the first and second were explicitly checked to
> not be NULL. Since the third expression is also required in these
> constructs it needs to be explicitly checked and rejected when missing.
> Otherwise the demangler will crash once it tries to d_print the
> NULL component. Added two examples to demangle-expected of strings
> that would crash before this fix.
>
> Found by American Fuzzy Lop (afl) fuzzer.
> ---
>  libiberty/ChangeLog                   | 7 +++++++
>  libiberty/cp-demangle.c               | 4 ++++
>  libiberty/testsuite/demangle-expected | 8 ++++++++
>  3 files changed, 19 insertions(+)
>
> diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
> index 41f3405..43617e4 100644
> --- a/libiberty/ChangeLog
> +++ b/libiberty/ChangeLog
> @@ -1,3 +1,10 @@
> +2016-11-15  Mark Wielaard  <mark@klomp.org>
> +
> +       * cp-demangle.c (d_expression_1): Make sure third expression
> +       exists for ?: and fold expressions.
> +       * testsuite/demangle-expected: Add examples of strings that could
> +       crash the demangler because of missing expression.
> +

This is not the approach usually taken by the demangler.  The usual
approach would be to use a different code, other than
DEMANGLE_COMPONENT_TRINARY_ARG2, that requires a non-NULL right
argument, and test for that in d_make_comp.  But I suppose this
approach is simple enough, so this patch is OK.  Thanks.

Ian
diff mbox

Patch

diff --git a/libiberty/ChangeLog b/libiberty/ChangeLog
index 41f3405..43617e4 100644
--- a/libiberty/ChangeLog
+++ b/libiberty/ChangeLog
@@ -1,3 +1,10 @@ 
+2016-11-15  Mark Wielaard  <mark@klomp.org>
+
+	* cp-demangle.c (d_expression_1): Make sure third expression
+	exists for ?: and fold expressions.
+	* testsuite/demangle-expected: Add examples of strings that could
+	crash the demangler because of missing expression.
+
 2016-11-14  Mark Wielaard  <mark@klomp.org>
 
 	* cplus-dem.c (demangle_signature): After 'H', template function,
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index e239155..45663fe 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -3415,6 +3415,8 @@  d_expression_1 (struct d_info *di)
 		first = d_expression_1 (di);
 		second = d_expression_1 (di);
 		third = d_expression_1 (di);
+		if (third == NULL)
+		  return NULL;
 	      }
 	    else if (code[0] == 'f')
 	      {
@@ -3422,6 +3424,8 @@  d_expression_1 (struct d_info *di)
 		first = d_operator_name (di);
 		second = d_expression_1 (di);
 		third = d_expression_1 (di);
+		if (third == NULL)
+		  return NULL;
 	      }
 	    else if (code[0] == 'n')
 	      {
diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected
index 236161c..af491d8 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -4626,3 +4626,11 @@  _$_H1R
 # Could crash
 _Q8ccQ4M2e.
 _Q8ccQ4M2e.
+
+# fold-expression with missing third component could crash.
+_Z12binary_rightIJLi1ELi2ELi3EEEv1AIXfRplT_LiEEE
+_Z12binary_rightIJLi1ELi2ELi3EEEv1AIXfRplT_LiEEE
+
+# ?: expression with missing third component could crash.
+AquT_quT_4mxautouT_4mxxx
+AquT_quT_4mxautouT_4mxxx