[PATCH] tree.c: avoid segfault with closing parenthesis ')' after -files0-from

Bernhard Voelker <[email protected]>
Newsgroups gmane.comp.gnu.findutils.patches,gmane.comp.gnu.findutils.bugs
Message-ID <[email protected]>
Prompted by the following warning of GCC-11.1.1:

  tree.c: In function 'get_expr':
  tree.c:140:31: warning: dereference of NULL 'prev_pred' [CWE-476] \
  [-Wanalyzer-null-dereference]
    140 |       if ((UNI_OP == prev_pred->p_type
        |                      ~~~~~~~~~^~~~~~~~

Former versions of find are not vulnerable to this, because a closing
parenthesis ')' without anything before was treated as a pathname
rather than an option.
But this is possible now with the recent introduction of the -files0-from
option (see commit 11576f4e6a) - yet still an invalid invocation.
Reproducer for a crash:
  $ find -files0-from FILE ')' -print
  Segmentation fault (core dumped)

In the similar case when the user didn't specify any action, and find(1)
adds the default action via '( user-expr ) -print', the error diagnostic
was very confusing, too:
  $ find -files0-from FILE ')'
  find: invalid expression; empty parentheses are not allowed.

* find/tree.c (get_expr): Handle the cases when there's no predicate
before CLOSE_PAREN, and output a useful error diagnostic.
* tests/find/files0-from.sh: Add a test case for it.
---
 find/tree.c               | 14 ++++++++++++++
 tests/find/files0-from.sh | 13 +++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/find/tree.c b/find/tree.c
index 1b33edc7..39c1a4ce 100644
--- a/find/tree.c
+++ b/find/tree.c
@@ -128,6 +128,14 @@ get_expr (struct predicate **input,
       break;
 
     case CLOSE_PAREN:
+      if (prev_pred == NULL)
+	{
+	  /* Happens with e.g. "find -files0-from - ')' -print" */
+	  die (EXIT_FAILURE, 0,
+	       _("invalid expression: expected expression before closing parentheses '%s'."),
+	       this_pred->p_name);
+	}
+
       if ((UNI_OP == prev_pred->p_type
 	  || BI_OP == prev_pred->p_type)
 	  && !this_pred->artificial)
@@ -180,6 +188,12 @@ get_expr (struct predicate **input,
       *input = (*input)->pred_next;
       if ( (*input)->p_type == CLOSE_PAREN )
 	{
+	  if (prev_pred->artificial)
+	    {
+	      die (EXIT_FAILURE, 0,
+		   _("invalid expression: expected expression before closing parentheses '%s'."),
+		   (*input)->p_name);
+	    }
 	  die (EXIT_FAILURE, 0,
 	       _("invalid expression; empty parentheses are not allowed."));
 	}
diff --git a/tests/find/files0-from.sh b/tests/find/files0-from.sh
index b930a999..bf54a1b2 100755
--- a/tests/find/files0-from.sh
+++ b/tests/find/files0-from.sh
@@ -108,6 +108,19 @@ cat /dev/null | returns_ 1 find -files0-from - > out 2> err || fail=1
 compare /dev/null out || fail=1
 grep 'file with starting points is empty:.*standard input' err || fail=1
 
+# With the -files0-from option, now a closing paren could be passed as first
+# predicate (without, it is treated as a starting point).  Ensure that find(1)
+# handles this invalid invocation.
+returns_ 1 find -files0-from - ')' -print < /dev/null > out 2> err || fail=1
+compare /dev/null out || fail=1
+grep "inv.*: expected expression before closing parentheses" err || fail=1
+
+# Likewise in the case find(1) implicitly adds the default action via the
+# artificial '( user-expr ) -print' construct.
+returns_ 1 find -files0-from - ')' < /dev/null > out 2> err || fail=1
+compare /dev/null out || fail=1
+grep "inv.*: expected expression before closing parentheses" err || fail=1
+
 # Now a regular case: 2 files: expect the same output.
 touch a b || framework_failure_
 printf '%s\0' a b > in || framework_failure_
-- 
2.32.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.