posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368)

Message ID 20260422122514.603156-1-adhemerval.zanella@linaro.org (mailing list archive)
State New
Headers
Series posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368) |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686

Commit Message

Adhemerval Zanella Netto April 22, 2026, 12:24 p.m. UTC
  The previous implementation saved a copy of the wordexp_t struct at
entry and blindly restored it on error via (*pwordexp = old_word).
This is incorrect when WRDE_APPEND is set because w_addword may have
called realloc on we_wordv during partial processing before the error
was detected.  If realloc relocated the buffer, the saved we_wordv
pointer is dangling; restoring it causes a use-after-free in the
caller (e.g. via wordfree), and the relocated buffer is leaked.

Fix this by duplicating the we_wordv pointer array at entry when
WRDE_APPEND is set, so that all subsequent realloc calls inside
w_addword operate on the copy.

This change also fixes a POSIX conformance issue: if the WRDE_APPEND
flag is specified, pwordexp->we_wordc and pwordexp->we_wordv shall
not be modified.

Also fix two pre-existing error return paths in the '"' and '\'' cases
that returned directly from w_addword failures instead of going through
do_error, which would leak the saved array (and previously would also
skip the word cleanup).

A new test (tst-wordexp-append) exercises the changes:

- we_wordc and we_wordv pointer preserved on WRDE_BADCHAR/WRDE_SYNTAX
- we_wordv pointer stability when realloc relocates the buffer
- original words remain accessible after a failed append
- successful append still works (regression)
- recovery: successful append after a failed one
- repeated failures do not corrupt the state
- error without WRDE_APPEND still cleans up properly
- WRDE_BADCHAR on the first character (no partial w_addword)
- WRDE_APPEND into an empty (zeroed) wordexp_t

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 posix/Makefile             |   1 +
 posix/tst-wordexp-append.c | 307 +++++++++++++++++++++++++++++++++++++
 posix/wordexp.c            |  64 +++++++-
 3 files changed, 364 insertions(+), 8 deletions(-)
 create mode 100644 posix/tst-wordexp-append.c
  

Patch

diff --git a/posix/Makefile b/posix/Makefile
index a5e5162c61..1ee73b0b65 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -329,6 +329,7 @@  tests := \
   tst-wait3 \
   tst-wait4 \
   tst-waitid \
+  tst-wordexp-append \
   tst-wordexp-nocmd \
   tst-wordexp-reuse \
   tstgetopt \
diff --git a/posix/tst-wordexp-append.c b/posix/tst-wordexp-append.c
new file mode 100644
index 0000000000..6253520c57
--- /dev/null
+++ b/posix/tst-wordexp-append.c
@@ -0,0 +1,307 @@ 
+/* Test for wordexp with WRDE_APPEND flag.
+   Copyright (C) 2026 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <wordexp.h>
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <support/check.h>
+#include <support/support.h>
+
+/* Attempt to force realloc to relocate the we_wordv buffer by placing an
+   allocation right after it.  Returns a pointer that must be freed after
+   the test.  */
+static void *
+place_blocker (void)
+{
+  void *p = xmalloc (0x1000);
+  /* Write to it so the compiler cannot optimize it away and the allocator
+     actually commits the pages.  */
+  memset (p, 0x41, 0x1000);
+  return p;
+}
+
+/* Verify that all words in we match the expected NULL-terminated
+   array.  */
+static void
+check_words (const wordexp_t *we, const char *const *expected, int line)
+{
+  size_t i;
+  for (i = 0; expected[i] != NULL; i++)
+    {
+      TEST_VERIFY (i < we->we_wordc);
+      TEST_COMPARE_STRING (we->we_wordv[we->we_offs + i], expected[i]);
+    }
+  TEST_COMPARE (we->we_wordc, i);
+}
+
+#define CHECK_WORDS(we, ...) \
+  do {								\
+    const char *const expected_[] = { __VA_ARGS__, NULL };	\
+    check_words (we, expected_, __LINE__);			\
+  } while (0)
+
+/* Test 1: WRDE_APPEND + WRDE_BADCHAR preserves we_wordc.  */
+static void
+test_append_badchar_preserves_count (void)
+{
+  printf ("info: test_append_badchar_preserves_count\n");
+  wordexp_t we = { 0 };
+
+  TEST_COMPARE (wordexp ("one two three", &we, 0), 0);
+  TEST_COMPARE (we.we_wordc, 3);
+
+  size_t saved_count = we.we_wordc;
+
+  /* ')' triggers WRDE_BADCHAR and  "extra" would be a new word if the
+     expansion succeeded, exercising the w_addword path before the error
+     is detected.  */
+  TEST_COMPARE (wordexp ("extra )", &we, WRDE_APPEND), WRDE_BADCHAR);
+  TEST_COMPARE (we.we_wordc, saved_count);
+
+  wordfree (&we);
+}
+
+/* Test 2: WRDE_APPEND + WRDE_BADCHAR preserves the we_wordv pointer even
+   when internal realloc would move the buffer.  */
+static void
+test_append_badchar_preserves_pointer (void)
+{
+  printf ("info: test_append_badchar_preserves_pointer\n");
+  wordexp_t we = { 0 };
+
+  /* Use many words so that the initial we_wordv allocation is
+     non-trivial and a later realloc is more likely to move it.  */
+  TEST_COMPARE (wordexp ("a b c d e f g h", &we, 0), 0);
+  TEST_COMPARE (we.we_wordc, 8);
+
+  char **saved_wordv = we.we_wordv;
+  size_t saved_count = we.we_wordc;
+
+  /* Place an allocation right after to make realloc move the buffer.  */
+  void *blocker = place_blocker ();
+
+  TEST_COMPARE (wordexp ("append )", &we, WRDE_APPEND), WRDE_BADCHAR);
+  TEST_COMPARE (we.we_wordc, saved_count);
+  TEST_VERIFY (we.we_wordv == saved_wordv);
+
+  free (blocker);
+  wordfree (&we);
+}
+
+/* Test 3: After a failed WRDE_APPEND the original words are still accessible
+   and correct.  */
+static void
+test_append_badchar_words_intact (void)
+{
+  printf ("info: test_append_badchar_words_intact\n");
+  wordexp_t we = { 0 };
+
+  TEST_COMPARE (wordexp ("alpha beta gamma", &we, 0), 0);
+  CHECK_WORDS (&we, "alpha", "beta", "gamma");
+
+  void *blocker = place_blocker ();
+  TEST_COMPARE (wordexp ("delta )", &we, WRDE_APPEND), WRDE_BADCHAR);
+  free (blocker);
+
+  /* Words must still be intact.  */
+  CHECK_WORDS (&we, "alpha", "beta", "gamma");
+  /* The NULL terminator must still be present.  */
+  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
+
+  wordfree (&we);
+}
+
+/* Test 4: Successful WRDE_APPEND still works (regression test).  */
+static void
+test_append_success (void)
+{
+  printf ("info: test_append_success\n");
+  wordexp_t we = { 0 };
+
+  TEST_COMPARE (wordexp ("hello", &we, 0), 0);
+  TEST_COMPARE (we.we_wordc, 1);
+
+  TEST_COMPARE (wordexp ("world", &we, WRDE_APPEND), 0);
+  TEST_COMPARE (we.we_wordc, 2);
+  CHECK_WORDS (&we, "hello", "world");
+
+  wordfree (&we);
+}
+
+/* Test 5: Successful append after a failed append — the implementation must
+   recover and allow further use of the wordexp_t.  */
+static void
+test_append_success_after_failure (void)
+{
+  printf ("info: test_append_success_after_failure\n");
+  wordexp_t we = { 0 };
+
+  TEST_COMPARE (wordexp ("first", &we, 0), 0);
+  CHECK_WORDS (&we, "first");
+
+  void *blocker = place_blocker ();
+  TEST_COMPARE (wordexp ("bad |", &we, WRDE_APPEND), WRDE_BADCHAR);
+  free (blocker);
+
+  /* State must be exactly as before the failed call.  */
+  CHECK_WORDS (&we, "first");
+
+  /* A subsequent successful append must work.  */
+  TEST_COMPARE (wordexp ("second third", &we, WRDE_APPEND), 0);
+  CHECK_WORDS (&we, "first", "second", "third");
+
+  wordfree (&we);
+}
+
+/* Test 6: Multiple consecutive failed appends do not corrupt state.  */
+static void
+test_append_multiple_failures (void)
+{
+  printf ("info: test_append_multiple_failures\n");
+  wordexp_t we = { 0 };
+
+  TEST_COMPARE (wordexp ("keep this", &we, 0), 0);
+  CHECK_WORDS (&we, "keep", "this");
+
+  size_t saved_count = we.we_wordc;
+  char **saved_wordv = we.we_wordv;
+
+  void *blocker = place_blocker ();
+
+  /* Each of these bad characters must leave the state unchanged.  */
+  TEST_COMPARE (wordexp ("x )", &we, WRDE_APPEND), WRDE_BADCHAR);
+  TEST_COMPARE (wordexp ("x |", &we, WRDE_APPEND), WRDE_BADCHAR);
+  TEST_COMPARE (wordexp ("x ;", &we, WRDE_APPEND), WRDE_BADCHAR);
+  TEST_COMPARE (wordexp ("x &", &we, WRDE_APPEND), WRDE_BADCHAR);
+  TEST_COMPARE (wordexp ("x <", &we, WRDE_APPEND), WRDE_BADCHAR);
+  TEST_COMPARE (wordexp ("x >", &we, WRDE_APPEND), WRDE_BADCHAR);
+
+  free (blocker);
+
+  TEST_COMPARE (we.we_wordc, saved_count);
+  TEST_VERIFY (we.we_wordv == saved_wordv);
+  CHECK_WORDS (&we, "keep", "this");
+
+  wordfree (&we);
+}
+
+/* Test 7: WRDE_APPEND with WRDE_SYNTAX error (unterminated quote) also
+   preserves state.  */
+static void
+test_append_syntax_error (void)
+{
+  printf ("info: test_append_syntax_error\n");
+  wordexp_t we = { 0 };
+
+  TEST_COMPARE (wordexp ("original", &we, 0), 0);
+  CHECK_WORDS (&we, "original");
+
+  char **saved_wordv = we.we_wordv;
+  size_t saved_count = we.we_wordc;
+
+  void *blocker = place_blocker ();
+  /* Unterminated double quote triggers WRDE_SYNTAX.  */
+  TEST_COMPARE (wordexp ("\"unterminated", &we, WRDE_APPEND), WRDE_SYNTAX);
+  free (blocker);
+
+  TEST_COMPARE (we.we_wordc, saved_count);
+  TEST_VERIFY (we.we_wordv == saved_wordv);
+  CHECK_WORDS (&we, "original");
+
+  wordfree (&we);
+}
+
+/* Test 8: Error without WRDE_APPEND still works (regression test for the
+   non-APPEND code path in do_error).  */
+static void
+test_no_append_error (void)
+{
+  printf ("info: test_no_append_error\n");
+  wordexp_t we = { 0 };
+
+  /* Simple failure without WRDE_APPEND.  */
+  TEST_COMPARE (wordexp ("bad |", &we, 0), WRDE_BADCHAR);
+
+  /* After failure without WRDE_APPEND the struct should be safe to
+     reuse — start fresh.  */
+  TEST_COMPARE (wordexp ("ok", &we, 0), 0);
+  CHECK_WORDS (&we, "ok");
+
+  wordfree (&we);
+}
+
+/* Test 9: WRDE_BADCHAR on the very first character (no partial words added
+   before the error).  */
+static void
+test_append_badchar_immediate (void)
+{
+  printf ("info: test_append_badchar_immediate\n");
+  wordexp_t we = { 0 };
+
+  TEST_COMPARE (wordexp ("hello world", &we, 0), 0);
+  CHECK_WORDS (&we, "hello", "world");
+
+  char **saved_wordv = we.we_wordv;
+  size_t saved_count = we.we_wordc;
+
+  /* The bad character is the very first byte — no w_addword call happens
+     before the error.  */
+  TEST_COMPARE (wordexp ("|", &we, WRDE_APPEND), WRDE_BADCHAR);
+  TEST_COMPARE (we.we_wordc, saved_count);
+  TEST_VERIFY (we.we_wordv == saved_wordv);
+
+  wordfree (&we);
+}
+
+/* Test 10: WRDE_APPEND into an empty wordexp_t (initial call uses WRDE_APPEND
+   with a zeroed struct — unusual but allowed).  */
+static void
+test_append_into_empty (void)
+{
+  printf ("info: test_append_into_empty\n");
+  wordexp_t we = { 0 };
+
+  /* First call with WRDE_APPEND on a zeroed struct.  The implementation
+     must handle we_wordv == NULL gracefully.  */
+  TEST_COMPARE (wordexp ("solo", &we, WRDE_APPEND), 0);
+  TEST_COMPARE (we.we_wordc, 1);
+  CHECK_WORDS (&we, "solo");
+
+  wordfree (&we);
+}
+
+static int
+do_test (void)
+{
+  test_append_badchar_preserves_count ();
+  test_append_badchar_preserves_pointer ();
+  test_append_badchar_words_intact ();
+  test_append_success ();
+  test_append_success_after_failure ();
+  test_append_multiple_failures ();
+  test_append_syntax_error ();
+  test_no_append_error ();
+  test_append_badchar_immediate ();
+  test_append_into_empty ();
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/posix/wordexp.c b/posix/wordexp.c
index 4a8541add4..119ce05d9c 100644
--- a/posix/wordexp.c
+++ b/posix/wordexp.c
@@ -35,6 +35,7 @@ 
 #include <scratch_buffer.h>
 #include <_itoa.h>
 #include <assert.h>
+#include <intprops.h>
 
 /*
  * This is a recursive-descent-style word expansion routine.
@@ -2212,6 +2213,12 @@  wordexp (const char *words, wordexp_t *pwordexp, int flags)
   char ifs_white[4];
   wordexp_t old_word = *pwordexp;
 
+  /* When WRDE_APPEND is set we work on a copy of the we_wordv array so that
+     the caller's original pointer is never invalidated by realloc inside
+     w_addword.  The saved_wordv keeps the original; on success we free it,
+     on non-NOSPACE error we free the working copy and restore the original.  */
+  char **saved_wordv = NULL;
+
   if (flags & WRDE_REUSE)
     {
       /* Minimal implementation of WRDE_REUSE for now */
@@ -2246,6 +2253,23 @@  wordexp (const char *words, wordexp_t *pwordexp, int flags)
 	  pwordexp->we_offs = 0;
 	}
     }
+  else if (pwordexp->we_wordv != NULL)
+    {
+      /* WRDE_APPEND with an existing word list: duplicate the array so that
+	 realloc during parsing does not invalidate the caller's pointer.  The
+	 strings themselves are shared.  */
+      size_t num_p;
+      char **dup;
+      if (INT_ADD_WRAPV (pwordexp->we_offs, pwordexp->we_wordc, &num_p)
+	  || INT_ADD_WRAPV (num_p, 1, &num_p))
+	return WRDE_NOSPACE;
+      dup = __libc_reallocarray (NULL, num_p, sizeof *dup);
+      if (dup == NULL)
+	return WRDE_NOSPACE;
+      memcpy (dup, pwordexp->we_wordv, num_p * sizeof *dup);
+      saved_wordv = pwordexp->we_wordv;
+      pwordexp->we_wordv = dup;
+    }
 
   /* Find out what the field separators are.
    * There are two types: whitespace and non-whitespace.
@@ -2326,7 +2350,7 @@  wordexp (const char *words, wordexp_t *pwordexp, int flags)
 	    error = w_addword (pwordexp, NULL);
 
 	    if (error)
-	      return error;
+	      goto do_error;
 	  }
 
 	break;
@@ -2344,7 +2368,7 @@  wordexp (const char *words, wordexp_t *pwordexp, int flags)
 	    error = w_addword (pwordexp, NULL);
 
 	    if (error)
-	      return error;
+	      goto do_error;
 	  }
 
 	break;
@@ -2410,10 +2434,15 @@  wordexp (const char *words, wordexp_t *pwordexp, int flags)
 
   /* There was a word separator at the end */
   if (word == NULL) /* i.e. w_newword */
-    return 0;
+    {
+      free (saved_wordv);
+      return 0;
+    }
 
   /* There was no field separator at the end */
-  return w_addword (pwordexp, word);
+  error = w_addword (pwordexp, word);
+  free (saved_wordv);
+  return error;
 
 do_error:
   /* Error:
@@ -2424,11 +2453,30 @@  do_error:
   free (word);
 
   if (error == WRDE_NOSPACE)
-    return WRDE_NOSPACE;
+    {
+      /* we_wordc and we_wordv are updated to reflect any words that were
+	 successfully expanded.  The old array is obsolete.  */
+      free (saved_wordv);
+      return WRDE_NOSPACE;
+    }
 
-  if ((flags & WRDE_APPEND) == 0)
-    wordfree (pwordexp);
+  if (flags & WRDE_APPEND)
+    {
+      /* POSIX 2024 states that for in other error cases, if the WRDE_APPEND
+	 flag was specified, we_wordc and we_wordv shall not be modified.
+
+	 Free strings appended during this call, discard the working copy of
+	 we_wordv, and restore the caller's original pointer.  */
+      while (pwordexp->we_wordc > old_word.we_wordc)
+	free (pwordexp->we_wordv[pwordexp->we_offs + --pwordexp->we_wordc]);
+      free (pwordexp->we_wordv);
+      pwordexp->we_wordv = saved_wordv;
+    }
+  else
+    {
+      wordfree (pwordexp);
+      *pwordexp = old_word;
+    }
 
-  *pwordexp = old_word;
   return error;
 }