Fix use-after-free in the argument list handling in the parser

The recently added vector comparison handling added code to better handle multiple argument lists in the same expression using a list of lists structure, but forgot to add protection against adding a the same list twice on that list of lists. Any reasonably complex expression with multiple function calls reveals a use-after-free bug in this code, of which many are in the unit tests already. But I forgot to run the whole test suite using ASAN/UBSAN before releasing 0.8.1. ¯\_(ツ)_/¯ Add the protection against duplicate entries on the same list. Signed-off-by: Zoltán Böszörményi <[email protected]>
zboszor · Nov 19, 2023 · c0a5cde · c0a5cde
1 parent b3444fd
commit c0a5cde
Showing 1 changed file with 20 additions and 8 deletions.
diff --git a/libsrc/ocrptgrammar.y b/libsrc/ocrptgrammar.y
@@ -55,6 +55,7 @@ static ocrpt_expr *newvectormatchexpr(yyscan_t yyscanner, ocrpt_string *fname, o
 static ocrpt_expr *makefuncexpr(yyscan_t yyscanner, ocrpt_expr *func, ocrpt_list *l);
 static ocrpt_expr *parseembeddedexpr(yyscan_t yyscanner, ocrpt_expr *func, ocrpt_list *l);
 static ocrpt_expr *ocrpt_expr_parse_internal(opencreport *o, ocrpt_report *r, const char *expr_string, char **err);
+static ocrpt_list *addtoargliststack(ocrpt_list *l, void *ptr);
 
 %}
 
@@ -406,17 +407,16 @@ argelems:
 								base_yy_extra_type *extra = parser_yyget_extra(yyscanner);
 								ocrpt_list *l = ocrpt_makelist1($1);
 
-								if (extra->parsed_arglist)
-									extra->parsed_arglist_stack = ocrpt_list_prepend(extra->parsed_arglist_stack, extra->parsed_arglist);
+								extra->parsed_arglist_stack = addtoargliststack(extra->parsed_arglist_stack, extra->parsed_arglist);
 
 								$$ = extra->parsed_arglist = l;
 							}
 	| argelems ',' exp		{
 								base_yy_extra_type *extra = parser_yyget_extra(yyscanner);
 								ocrpt_list *l = ocrpt_list_append($1, $3);
 
-								if (extra->parsed_arglist && l != extra->parsed_arglist)
-									extra->parsed_arglist_stack = ocrpt_list_prepend(extra->parsed_arglist_stack, extra->parsed_arglist);
+								if (l != extra->parsed_arglist)
+									extra->parsed_arglist_stack = addtoargliststack(extra->parsed_arglist_stack, extra->parsed_arglist);
 
 								$$ = extra->parsed_arglist = l;
 							}
@@ -426,16 +426,15 @@ arglist:
 	/* empty */				{
 								base_yy_extra_type *extra = parser_yyget_extra(yyscanner);
 
-								if (extra->parsed_arglist)
-									extra->parsed_arglist_stack = ocrpt_list_prepend(extra->parsed_arglist_stack, extra->parsed_arglist);
+								extra->parsed_arglist_stack = addtoargliststack(extra->parsed_arglist_stack, extra->parsed_arglist);
 
 								$$ = extra->parsed_arglist = NULL;
 							}
 	| argelems				{
 								base_yy_extra_type *extra = parser_yyget_extra(yyscanner);
 
-								if (extra->parsed_arglist && $1 != extra->parsed_arglist)
-									extra->parsed_arglist_stack = ocrpt_list_prepend(extra->parsed_arglist_stack, extra->parsed_arglist);
+								if ($1 != extra->parsed_arglist)
+									extra->parsed_arglist_stack = addtoargliststack(extra->parsed_arglist_stack, extra->parsed_arglist);
 
 								$$ = extra->parsed_arglist = $1;
 							}
@@ -824,3 +823,16 @@ static ocrpt_expr *parseembeddedexpr(yyscan_t yyscanner, ocrpt_expr *func, ocrpt
 
 	return e;
 }
+
+static ocrpt_list *addtoargliststack(ocrpt_list *l, void *ptr) {
+	if (!ptr)
+		return l;
+
+	bool found = false;
+
+	for (ocrpt_list *l1 = l; l1 && !found; l1 = l1->next)
+		if (l1->data == ptr)
+			found = true;
+
+	return found ? l : ocrpt_list_prepend(l, ptr);
+}