public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2] posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368)
@ 2026-07-01 18:51 Adhemerval Zanella
  2026-07-08 21:59 ` DJ Delorie
  0 siblings, 1 reply; 8+ messages in thread
From: Adhemerval Zanella @ 2026-07-01 18:51 UTC (permalink / raw)
  To: libc-alpha; +Cc: Florian Weimer

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).

Checked on x86_64-linux-gnu and i686-linux-gnu.
--
Changes from v1:
* Use aninterposed realloc to make the test more deterministic.
* Add WRDE_DOOFFS tests.
---
 posix/Makefile             |   1 +
 posix/tst-wordexp-append.c | 375 +++++++++++++++++++++++++++++++++++++
 posix/wordexp.c            |  64 ++++++-
 3 files changed, 432 insertions(+), 8 deletions(-)
 create mode 100644 posix/tst-wordexp-append.c

diff --git a/posix/Makefile b/posix/Makefile
index d9f897faa16..10d70ae0ae2 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -330,6 +330,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 00000000000..6aad7a0a8ed
--- /dev/null
+++ b/posix/tst-wordexp-append.c
@@ -0,0 +1,375 @@
+/* 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 <malloc.h>
+
+#include <support/check.h>
+#include <support/support.h>
+
+/* w_addword grows we_wordv with realloc, make every call guaranteed to
+   relocate the block.  This makes BZ 34090 regression more deterministic.  */
+void *
+realloc (void *ptr, size_t size)
+{
+  if (ptr == NULL)
+    return malloc (size);
+  if (size == 0)
+    {
+      free (ptr);
+      return NULL;
+    }
+
+  void *new = malloc (size);
+  if (new == NULL)
+    return NULL;
+
+  /* Copy only what is valid in the old block to avoid reading past it.  */
+  size_t old = malloc_usable_size (ptr);
+  memcpy (new, ptr, old < size ? old : size);
+  free (ptr);
+  return new;
+}
+
+/* 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;
+
+  /* The interposed realloc guarantees the internal we_wordv buffer moves
+     during parsing, so the pointer-stability check below is meaningful.  */
+  TEST_COMPARE (wordexp ("append )", &we, WRDE_APPEND), WRDE_BADCHAR);
+  TEST_COMPARE (we.we_wordc, saved_count);
+  TEST_VERIFY (we.we_wordv == saved_wordv);
+
+  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");
+
+  TEST_COMPARE (wordexp ("delta )", &we, WRDE_APPEND), WRDE_BADCHAR);
+
+  /* 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");
+
+  TEST_COMPARE (wordexp ("bad |", &we, WRDE_APPEND), WRDE_BADCHAR);
+
+  /* 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;
+
+  /* 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);
+
+  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;
+
+  /* Unterminated double quote triggers WRDE_SYNTAX.  */
+  TEST_COMPARE (wordexp ("\"unterminated", &we, WRDE_APPEND), WRDE_SYNTAX);
+
+  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);
+}
+
+/* Verify that the leading we_offs slots are all NULL.  */
+static void
+check_offs_null (const wordexp_t *we)
+{
+  for (size_t i = 0; i < we->we_offs; i++)
+    TEST_VERIFY (we->we_wordv[i] == NULL);
+}
+
+/* Test 11: successful WRDE_APPEND with WRDE_DOOFFS and a non-zero we_offs.
+   The leading offset slots must stay NULL and words must land at
+   we_wordv[we_offs + i] across both the initial and the appended call.  */
+static void
+test_dooffs_append_success (void)
+{
+  printf ("info: test_dooffs_append_success\n");
+  wordexp_t we = { 0 };
+  we.we_offs = 2;
+
+  TEST_COMPARE (wordexp ("one two", &we, WRDE_DOOFFS), 0);
+  TEST_COMPARE (we.we_offs, 2);
+  check_offs_null (&we);
+  CHECK_WORDS (&we, "one", "two");
+
+  TEST_COMPARE (wordexp ("three", &we, WRDE_APPEND | WRDE_DOOFFS), 0);
+  TEST_COMPARE (we.we_offs, 2);
+  check_offs_null (&we);
+  CHECK_WORDS (&we, "one", "two", "three");
+  /* The NULL terminator must sit right after the last word.  */
+  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
+
+  wordfree (&we);
+}
+
+/* Test 12: failed WRDE_APPEND with WRDE_DOOFFS preserves we_wordc, the
+   we_wordv pointer, the words and the leading NULL offset slots.  This
+   exercises the we_offs arithmetic in the array duplication and in the
+   error-path cleanup (we_wordv[we_offs + --we_wordc]).  */
+static void
+test_dooffs_append_error_preserves_state (void)
+{
+  printf ("info: test_dooffs_append_error_preserves_state\n");
+  wordexp_t we = { 0 };
+  we.we_offs = 3;
+
+  TEST_COMPARE (wordexp ("alpha beta", &we, WRDE_DOOFFS), 0);
+  check_offs_null (&we);
+  CHECK_WORDS (&we, "alpha", "beta");
+
+  char **saved_wordv = we.we_wordv;
+  size_t saved_count = we.we_wordc;
+
+  /* "gamma" is a partial word added via w_addword (forcing a relocating
+     realloc of we_wordv) before ')' triggers WRDE_BADCHAR.  */
+  TEST_COMPARE (wordexp ("gamma )", &we, WRDE_APPEND | WRDE_DOOFFS),
+		WRDE_BADCHAR);
+
+  TEST_COMPARE (we.we_offs, 3);
+  TEST_COMPARE (we.we_wordc, saved_count);
+  TEST_VERIFY (we.we_wordv == saved_wordv);
+  check_offs_null (&we);
+  CHECK_WORDS (&we, "alpha", "beta");
+  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
+
+  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 ();
+  test_dooffs_append_success ();
+  test_dooffs_append_error_preserves_state ();
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/posix/wordexp.c b/posix/wordexp.c
index f0f69ee85d5..5f3941fa845 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.
@@ -2224,6 +2225,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 */
@@ -2258,6 +2265,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.
@@ -2338,7 +2362,7 @@ wordexp (const char *words, wordexp_t *pwordexp, int flags)
 	    error = w_addword (pwordexp, NULL);
 
 	    if (error)
-	      return error;
+	      goto do_error;
 	  }
 
 	break;
@@ -2356,7 +2380,7 @@ wordexp (const char *words, wordexp_t *pwordexp, int flags)
 	    error = w_addword (pwordexp, NULL);
 
 	    if (error)
-	      return error;
+	      goto do_error;
 	  }
 
 	break;
@@ -2422,10 +2446,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:
@@ -2436,11 +2465,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;
 }
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368)
  2026-07-01 18:51 [PATCH v2] posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368) Adhemerval Zanella
@ 2026-07-08 21:59 ` DJ Delorie
  2026-07-13 18:11   ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 8+ messages in thread
From: DJ Delorie @ 2026-07-08 21:59 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, fweimer


A few tests could have more coverage, and one question about a return
path.

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> +/* w_addword grows we_wordv with realloc, make every call guaranteed to
> +   relocate the block.  This makes BZ 34090 regression more deterministic.  */
> +void *
> +realloc (void *ptr, size_t size)
> +{
> +  if (ptr == NULL)
> +    return malloc (size);
> +  if (size == 0)
> +    {
> +      free (ptr);
> +      return NULL;
> +    }
> +
> +  void *new = malloc (size);
> +  if (new == NULL)
> +    return NULL;
> +
> +  /* Copy only what is valid in the old block to avoid reading past it.  */
> +  size_t old = malloc_usable_size (ptr);
> +  memcpy (new, ptr, old < size ? old : size);
> +  free (ptr);
> +  return new;
> +}

Ok.  Might be useful to fill the old chunk with junk before freeing, but
I don't see how it would help this test.

> +/* 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);
> +}

What is the "line" argument here for?

> +#define CHECK_WORDS(we, ...) \
> +  do {								\
> +    const char *const expected_[] = { __VA_ARGS__, NULL };	\
> +    check_words (we, expected_, __LINE__);			\
> +  } while (0)

Ok.

> +/* 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);
> +}

Ok.

> +/* 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;
> +
> +  /* The interposed realloc guarantees the internal we_wordv buffer moves
> +     during parsing, so the pointer-stability check below is meaningful.  */
> +  TEST_COMPARE (wordexp ("append )", &we, WRDE_APPEND), WRDE_BADCHAR);
> +  TEST_COMPARE (we.we_wordc, saved_count);
> +  TEST_VERIFY (we.we_wordv == saved_wordv);
> +
> +  wordfree (&we);
> +}

The test should verify that realloc was actually called.

> +/* 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");
> +
> +  TEST_COMPARE (wordexp ("delta )", &we, WRDE_APPEND), WRDE_BADCHAR);
> +
> +  /* 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);
> +}

Ok.

> +/* 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);
> +}

Should test that the pointer actually changed, too.

> +/* 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");
> +
> +  TEST_COMPARE (wordexp ("bad |", &we, WRDE_APPEND), WRDE_BADCHAR);
> +
> +  /* 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);
> +}

Ok.

> +/* 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;
> +
> +  /* 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);
> +
> +  TEST_COMPARE (we.we_wordc, saved_count);
> +  TEST_VERIFY (we.we_wordv == saved_wordv);
> +  CHECK_WORDS (&we, "keep", "this");
> +
> +  wordfree (&we);
> +}

Ok.

> +/* 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;
> +
> +  /* Unterminated double quote triggers WRDE_SYNTAX.  */
> +  TEST_COMPARE (wordexp ("\"unterminated", &we, WRDE_APPEND), WRDE_SYNTAX);
> +
> +  TEST_COMPARE (we.we_wordc, saved_count);
> +  TEST_VERIFY (we.we_wordv == saved_wordv);
> +  CHECK_WORDS (&we, "original");
> +
> +  wordfree (&we);
> +}

Ok.

> +/* 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);
> +}

Should there be a test that "we" is unchanged?  I think the
wordexp/check is sufficient.  Ok.

> +/* 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);
> +}

Ok.

> +/* 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);
> +}

Ok.

> +/* Verify that the leading we_offs slots are all NULL.  */
> +static void
> +check_offs_null (const wordexp_t *we)
> +{
> +  for (size_t i = 0; i < we->we_offs; i++)
> +    TEST_VERIFY (we->we_wordv[i] == NULL);
> +}

Ok.

> +/* Test 11: successful WRDE_APPEND with WRDE_DOOFFS and a non-zero we_offs.
> +   The leading offset slots must stay NULL and words must land at
> +   we_wordv[we_offs + i] across both the initial and the appended call.  */
> +static void
> +test_dooffs_append_success (void)
> +{
> +  printf ("info: test_dooffs_append_success\n");
> +  wordexp_t we = { 0 };
> +  we.we_offs = 2;
> +
> +  TEST_COMPARE (wordexp ("one two", &we, WRDE_DOOFFS), 0);
> +  TEST_COMPARE (we.we_offs, 2);
> +  check_offs_null (&we);
> +  CHECK_WORDS (&we, "one", "two");
> +
> +  TEST_COMPARE (wordexp ("three", &we, WRDE_APPEND | WRDE_DOOFFS), 0);
> +  TEST_COMPARE (we.we_offs, 2);
> +  check_offs_null (&we);
> +  CHECK_WORDS (&we, "one", "two", "three");
> +  /* The NULL terminator must sit right after the last word.  */
> +  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
> +
> +  wordfree (&we);
> +}

Ok.

> +/* Test 12: failed WRDE_APPEND with WRDE_DOOFFS preserves we_wordc, the
> +   we_wordv pointer, the words and the leading NULL offset slots.  This
> +   exercises the we_offs arithmetic in the array duplication and in the
> +   error-path cleanup (we_wordv[we_offs + --we_wordc]).  */
> +static void
> +test_dooffs_append_error_preserves_state (void)
> +{
> +  printf ("info: test_dooffs_append_error_preserves_state\n");
> +  wordexp_t we = { 0 };
> +  we.we_offs = 3;
> +
> +  TEST_COMPARE (wordexp ("alpha beta", &we, WRDE_DOOFFS), 0);
> +  check_offs_null (&we);
> +  CHECK_WORDS (&we, "alpha", "beta");
> +
> +  char **saved_wordv = we.we_wordv;
> +  size_t saved_count = we.we_wordc;
> +
> +  /* "gamma" is a partial word added via w_addword (forcing a relocating
> +     realloc of we_wordv) before ')' triggers WRDE_BADCHAR.  */
> +  TEST_COMPARE (wordexp ("gamma )", &we, WRDE_APPEND | WRDE_DOOFFS),
> +		WRDE_BADCHAR);
> +
> +  TEST_COMPARE (we.we_offs, 3);
> +  TEST_COMPARE (we.we_wordc, saved_count);
> +  TEST_VERIFY (we.we_wordv == saved_wordv);
> +  check_offs_null (&we);
> +  CHECK_WORDS (&we, "alpha", "beta");
> +  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
> +
> +  wordfree (&we);
> +}

Ok.


> +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 ();
> +  test_dooffs_append_success ();
> +  test_dooffs_append_error_preserves_state ();
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

Ok.

> diff --git a/posix/wordexp.c b/posix/wordexp.c

> +  /* 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;

Ok.


> +  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);

I assume we rely on reallocarray to do its own overflow check, so...

> +      if (dup == NULL)
> +	return WRDE_NOSPACE;
> +      memcpy (dup, pwordexp->we_wordv, num_p * sizeof *dup);

We don't need to do it here.

> +      saved_wordv = pwordexp->we_wordv;
> +      pwordexp->we_wordv = dup;
> +    }

Ok.

>  	    error = w_addword (pwordexp, NULL);
>  
>  	    if (error)
> -	      return error;
> +	      goto do_error;
>  	  }

Ok.

>  	    if (error)
> -	      return error;
> +	      goto do_error;
>  	  }

Ok.

>  
>    /* There was a word separator at the end */
>    if (word == NULL) /* i.e. w_newword */
> -    return 0;
> +    {
> +      free (saved_wordv);
> +      return 0;
> +    }

Ok.

>    /* There was no field separator at the end */
> -  return w_addword (pwordexp, word);
> +  error = w_addword (pwordexp, word);
> +  free (saved_wordv);
> +  return error;

Is there a possible error here that would require us to preserve the
original array?

>    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;
> +    }

Ok.

> -  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;
>  }

Ok.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368)
  2026-07-08 21:59 ` DJ Delorie
@ 2026-07-13 18:11   ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 8+ messages in thread
From: Adhemerval Zanella Netto @ 2026-07-13 18:11 UTC (permalink / raw)
  To: DJ Delorie; +Cc: libc-alpha, fweimer



On 08/07/26 18:59, DJ Delorie wrote:
> 
> A few tests could have more coverage, and one question about a return
> path.
> 
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> +/* w_addword grows we_wordv with realloc, make every call guaranteed to
>> +   relocate the block.  This makes BZ 34090 regression more deterministic.  */
>> +void *
>> +realloc (void *ptr, size_t size)
>> +{
>> +  if (ptr == NULL)
>> +    return malloc (size);
>> +  if (size == 0)
>> +    {
>> +      free (ptr);
>> +      return NULL;
>> +    }
>> +
>> +  void *new = malloc (size);
>> +  if (new == NULL)
>> +    return NULL;
>> +
>> +  /* Copy only what is valid in the old block to avoid reading past it.  */
>> +  size_t old = malloc_usable_size (ptr);
>> +  memcpy (new, ptr, old < size ? old : size);
>> +  free (ptr);
>> +  return new;
>> +}
> 
> Ok.  Might be useful to fill the old chunk with junk before freeing, but
> I don't see how it would help this test.

Ack, I think it won't hurt and it might trigger invalid read if the code
read stale pointers.

> 
>> +/* 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);
>> +}
> 
> What is the "line" argument here for?

It was left-over from development, I will remove it (it helps a bit debugging
possible issues, but at same time I do not want to add another TEST_COMPARE_
to parametrize it or open-code the macro).

> 
>> +#define CHECK_WORDS(we, ...) \
>> +  do {								\
>> +    const char *const expected_[] = { __VA_ARGS__, NULL };	\
>> +    check_words (we, expected_, __LINE__);			\
>> +  } while (0)
> 
> Ok.
> 
>> +/* 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);
>> +}
> 
> Ok.
> 
>> +/* 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;
>> +
>> +  /* The interposed realloc guarantees the internal we_wordv buffer moves
>> +     during parsing, so the pointer-stability check below is meaningful.  */
>> +  TEST_COMPARE (wordexp ("append )", &we, WRDE_APPEND), WRDE_BADCHAR);
>> +  TEST_COMPARE (we.we_wordc, saved_count);
>> +  TEST_VERIFY (we.we_wordv == saved_wordv);
>> +
>> +  wordfree (&we);
>> +}
> 
> The test should verify that realloc was actually called.

Ack, I will a test for that.

> 
>> +/* 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");
>> +
>> +  TEST_COMPARE (wordexp ("delta )", &we, WRDE_APPEND), WRDE_BADCHAR);
>> +
>> +  /* 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);
>> +}
> 
> Ok.
> 
>> +/* 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);
>> +}
> 
> Should test that the pointer actually changed, too.

Ack.

> 
>> +/* 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");
>> +
>> +  TEST_COMPARE (wordexp ("bad |", &we, WRDE_APPEND), WRDE_BADCHAR);
>> +
>> +  /* 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);
>> +}
> 
> Ok.
> 
>> +/* 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;
>> +
>> +  /* 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);
>> +
>> +  TEST_COMPARE (we.we_wordc, saved_count);
>> +  TEST_VERIFY (we.we_wordv == saved_wordv);
>> +  CHECK_WORDS (&we, "keep", "this");
>> +
>> +  wordfree (&we);
>> +}
> 
> Ok.
> 
>> +/* 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;
>> +
>> +  /* Unterminated double quote triggers WRDE_SYNTAX.  */
>> +  TEST_COMPARE (wordexp ("\"unterminated", &we, WRDE_APPEND), WRDE_SYNTAX);
>> +
>> +  TEST_COMPARE (we.we_wordc, saved_count);
>> +  TEST_VERIFY (we.we_wordv == saved_wordv);
>> +  CHECK_WORDS (&we, "original");
>> +
>> +  wordfree (&we);
>> +}
> 
> Ok.
> 
>> +/* 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);
>> +}
> 
> Should there be a test that "we" is unchanged?  I think the
> wordexp/check is sufficient.  Ok.

Agreed.

> 
>> +/* 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);
>> +}
> 
> Ok.
> 
>> +/* 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);
>> +}
> 
> Ok.
> 
>> +/* Verify that the leading we_offs slots are all NULL.  */
>> +static void
>> +check_offs_null (const wordexp_t *we)
>> +{
>> +  for (size_t i = 0; i < we->we_offs; i++)
>> +    TEST_VERIFY (we->we_wordv[i] == NULL);
>> +}
> 
> Ok.
> 
>> +/* Test 11: successful WRDE_APPEND with WRDE_DOOFFS and a non-zero we_offs.
>> +   The leading offset slots must stay NULL and words must land at
>> +   we_wordv[we_offs + i] across both the initial and the appended call.  */
>> +static void
>> +test_dooffs_append_success (void)
>> +{
>> +  printf ("info: test_dooffs_append_success\n");
>> +  wordexp_t we = { 0 };
>> +  we.we_offs = 2;
>> +
>> +  TEST_COMPARE (wordexp ("one two", &we, WRDE_DOOFFS), 0);
>> +  TEST_COMPARE (we.we_offs, 2);
>> +  check_offs_null (&we);
>> +  CHECK_WORDS (&we, "one", "two");
>> +
>> +  TEST_COMPARE (wordexp ("three", &we, WRDE_APPEND | WRDE_DOOFFS), 0);
>> +  TEST_COMPARE (we.we_offs, 2);
>> +  check_offs_null (&we);
>> +  CHECK_WORDS (&we, "one", "two", "three");
>> +  /* The NULL terminator must sit right after the last word.  */
>> +  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
>> +
>> +  wordfree (&we);
>> +}
> 
> Ok.
> 
>> +/* Test 12: failed WRDE_APPEND with WRDE_DOOFFS preserves we_wordc, the
>> +   we_wordv pointer, the words and the leading NULL offset slots.  This
>> +   exercises the we_offs arithmetic in the array duplication and in the
>> +   error-path cleanup (we_wordv[we_offs + --we_wordc]).  */
>> +static void
>> +test_dooffs_append_error_preserves_state (void)
>> +{
>> +  printf ("info: test_dooffs_append_error_preserves_state\n");
>> +  wordexp_t we = { 0 };
>> +  we.we_offs = 3;
>> +
>> +  TEST_COMPARE (wordexp ("alpha beta", &we, WRDE_DOOFFS), 0);
>> +  check_offs_null (&we);
>> +  CHECK_WORDS (&we, "alpha", "beta");
>> +
>> +  char **saved_wordv = we.we_wordv;
>> +  size_t saved_count = we.we_wordc;
>> +
>> +  /* "gamma" is a partial word added via w_addword (forcing a relocating
>> +     realloc of we_wordv) before ')' triggers WRDE_BADCHAR.  */
>> +  TEST_COMPARE (wordexp ("gamma )", &we, WRDE_APPEND | WRDE_DOOFFS),
>> +		WRDE_BADCHAR);
>> +
>> +  TEST_COMPARE (we.we_offs, 3);
>> +  TEST_COMPARE (we.we_wordc, saved_count);
>> +  TEST_VERIFY (we.we_wordv == saved_wordv);
>> +  check_offs_null (&we);
>> +  CHECK_WORDS (&we, "alpha", "beta");
>> +  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
>> +
>> +  wordfree (&we);
>> +}
> 
> Ok.
> 
> 
>> +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 ();
>> +  test_dooffs_append_success ();
>> +  test_dooffs_append_error_preserves_state ();
>> +
>> +  return 0;
>> +}
>> +
>> +#include <support/test-driver.c>
> 
> Ok.
> 
>> diff --git a/posix/wordexp.c b/posix/wordexp.c
> 
>> +  /* 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;
> 
> Ok.
> 
> 
>> +  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);
> 
> I assume we rely on reallocarray to do its own overflow check, so...
> 
>> +      if (dup == NULL)
>> +	return WRDE_NOSPACE;
>> +      memcpy (dup, pwordexp->we_wordv, num_p * sizeof *dup);
> 
> We don't need to do it here.
> 
>> +      saved_wordv = pwordexp->we_wordv;
>> +      pwordexp->we_wordv = dup;
>> +    }
> 
> Ok.
> 
>>  	    error = w_addword (pwordexp, NULL);
>>  
>>  	    if (error)
>> -	      return error;
>> +	      goto do_error;
>>  	  }
> 
> Ok.
> 
>>  	    if (error)
>> -	      return error;
>> +	      goto do_error;
>>  	  }
> 
> Ok.
> 
>>  
>>    /* There was a word separator at the end */
>>    if (word == NULL) /* i.e. w_newword */
>> -    return 0;
>> +    {
>> +      free (saved_wordv);
>> +      return 0;
>> +    }
> 
> Ok.
> 
>>    /* There was no field separator at the end */
>> -  return w_addword (pwordexp, word);
>> +  error = w_addword (pwordexp, word);
>> +  free (saved_wordv);
>> +  return error;
> 
> Is there a possible error here that would require us to preserve the
> original array?

I don't think so, w_addword can only fail with WRDE_NOSPACE, and for it the
words successfully expanded are keppt.  However, double checking this
w_addword call might leak word on failure, I will fix it. 
> 
>>    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;
>> +    }
> 
> Ok.
> 
>> -  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;
>>  }
> 
> Ok.
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368)
  2026-08-12 14:10 ` Andreas Schwab
@ 2026-08-12 14:23   ` Adhemerval Zanella Netto
  0 siblings, 0 replies; 8+ messages in thread
From: Adhemerval Zanella Netto @ 2026-08-12 14:23 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha, DJ Delorie, Andreas K . Huettel



On 12/08/26 11:10, Andreas Schwab wrote:
> On Jul 13 2026, Adhemerval Zanella wrote:
> 
>> @@ -2258,6 +2265,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;
> 
> This can never happen, since it is exactly the size of the array to
> duplicate.
> 

Right, WRDE_APPEND contract with a non-NULL we_wordv should always come from 
previous successful wordexp call.  I think I got too defensive here, I will send
a cleanup to simplify this. 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368)
  2026-07-13 18:16 Adhemerval Zanella
  2026-07-13 18:18 ` Adhemerval Zanella Netto
  2026-07-13 18:53 ` DJ Delorie
@ 2026-08-12 14:10 ` Andreas Schwab
  2026-08-12 14:23   ` Adhemerval Zanella Netto
  2 siblings, 1 reply; 8+ messages in thread
From: Andreas Schwab @ 2026-08-12 14:10 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, DJ Delorie, Andreas K . Huettel

On Jul 13 2026, Adhemerval Zanella wrote:

> @@ -2258,6 +2265,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;

This can never happen, since it is exactly the size of the array to
duplicate.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368)
  2026-07-13 18:16 Adhemerval Zanella
  2026-07-13 18:18 ` Adhemerval Zanella Netto
@ 2026-07-13 18:53 ` DJ Delorie
  2026-08-12 14:10 ` Andreas Schwab
  2 siblings, 0 replies; 8+ messages in thread
From: DJ Delorie @ 2026-07-13 18:53 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, dilfridge


LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368)
  2026-07-13 18:16 Adhemerval Zanella
@ 2026-07-13 18:18 ` Adhemerval Zanella Netto
  2026-07-13 18:53 ` DJ Delorie
  2026-08-12 14:10 ` Andreas Schwab
  2 siblings, 0 replies; 8+ messages in thread
From: Adhemerval Zanella Netto @ 2026-07-13 18:18 UTC (permalink / raw)
  To: libc-alpha; +Cc: DJ Delorie, Andreas K . Huettel



On 13/07/26 15:16, Adhemerval Zanella wrote:
> 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).
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.

Sigh, it should v3 instead of v2.

> ---
>  posix/Makefile             |   1 +
>  posix/tst-wordexp-append.c | 393 +++++++++++++++++++++++++++++++++++++
>  posix/wordexp.c            |  69 ++++++-
>  3 files changed, 454 insertions(+), 9 deletions(-)
>  create mode 100644 posix/tst-wordexp-append.c
> 
> diff --git a/posix/Makefile b/posix/Makefile
> index d9f897faa16..10d70ae0ae2 100644
> --- a/posix/Makefile
> +++ b/posix/Makefile
> @@ -330,6 +330,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 00000000000..87f388f0a7e
> --- /dev/null
> +++ b/posix/tst-wordexp-append.c
> @@ -0,0 +1,393 @@
> +/* 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 <malloc.h>
> +
> +#include <support/check.h>
> +#include <support/support.h>
> +
> +static unsigned int relocating_reallocs;
> +
> +/* w_addword grows we_wordv with realloc, make every call guaranteed to
> +   relocate the block.  This makes BZ 34090 regression more deterministic.  */
> +void *
> +realloc (void *ptr, size_t size)
> +{
> +  if (ptr == NULL)
> +    return malloc (size);
> +  if (size == 0)
> +    {
> +      free (ptr);
> +      return NULL;
> +    }
> +
> +  void *new = malloc (size);
> +  if (new == NULL)
> +    return NULL;
> +
> +  /* Copy only what is valid in the old block to avoid reading past it.  */
> +  size_t old = malloc_usable_size (ptr);
> +  memcpy (new, ptr, old < size ? old : size);
> +  /* Clobber the old block so that a stale we_wordv pointer restored on the
> +     error path reads garbage instead of the old contents, which might
> +     otherwise survive intact and mask the bug.  */
> +  memset (ptr, 0x5a, old);
> +  free (ptr);
> +  relocating_reallocs++;
> +  return new;
> +}
> +
> +/* Verify that all words in we match the expected NULL-terminated
> +   array.  */
> +static void
> +check_words (const wordexp_t *we, const char *const *expected)
> +{
> +  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_);				\
> +  } 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;
> +  unsigned int saved_reallocs = relocating_reallocs;
> +
> +  /* The interposed realloc guarantees the internal we_wordv buffer moves
> +     during parsing, so the pointer-stability check below is meaningful.  */
> +  TEST_COMPARE (wordexp ("append )", &we, WRDE_APPEND), WRDE_BADCHAR);
> +  /* Verify that a relocating realloc actually happened during the failed
> +     call, otherwise the pointer-stability check is vacuous.  */
> +  TEST_VERIFY (relocating_reallocs > saved_reallocs);
> +  TEST_COMPARE (we.we_wordc, saved_count);
> +  TEST_VERIFY (we.we_wordv == saved_wordv);
> +
> +  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");
> +
> +  TEST_COMPARE (wordexp ("delta )", &we, WRDE_APPEND), WRDE_BADCHAR);
> +
> +  /* 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);
> +
> +  char **saved_wordv = we.we_wordv;
> +
> +  TEST_COMPARE (wordexp ("world", &we, WRDE_APPEND), 0);
> +  TEST_COMPARE (we.we_wordc, 2);
> +  /* A successful append works on a fresh copy of the array, so the
> +     caller-visible pointer must have changed.  */
> +  TEST_VERIFY (we.we_wordv != saved_wordv);
> +  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");
> +
> +  TEST_COMPARE (wordexp ("bad |", &we, WRDE_APPEND), WRDE_BADCHAR);
> +
> +  /* 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;
> +
> +  /* 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);
> +
> +  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;
> +
> +  /* Unterminated double quote triggers WRDE_SYNTAX.  */
> +  TEST_COMPARE (wordexp ("\"unterminated", &we, WRDE_APPEND), WRDE_SYNTAX);
> +
> +  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);
> +}
> +
> +/* Verify that the leading we_offs slots are all NULL.  */
> +static void
> +check_offs_null (const wordexp_t *we)
> +{
> +  for (size_t i = 0; i < we->we_offs; i++)
> +    TEST_VERIFY (we->we_wordv[i] == NULL);
> +}
> +
> +/* Test 11: successful WRDE_APPEND with WRDE_DOOFFS and a non-zero we_offs.
> +   The leading offset slots must stay NULL and words must land at
> +   we_wordv[we_offs + i] across both the initial and the appended call.  */
> +static void
> +test_dooffs_append_success (void)
> +{
> +  printf ("info: test_dooffs_append_success\n");
> +  wordexp_t we = { 0 };
> +  we.we_offs = 2;
> +
> +  TEST_COMPARE (wordexp ("one two", &we, WRDE_DOOFFS), 0);
> +  TEST_COMPARE (we.we_offs, 2);
> +  check_offs_null (&we);
> +  CHECK_WORDS (&we, "one", "two");
> +
> +  TEST_COMPARE (wordexp ("three", &we, WRDE_APPEND | WRDE_DOOFFS), 0);
> +  TEST_COMPARE (we.we_offs, 2);
> +  check_offs_null (&we);
> +  CHECK_WORDS (&we, "one", "two", "three");
> +  /* The NULL terminator must sit right after the last word.  */
> +  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
> +
> +  wordfree (&we);
> +}
> +
> +/* Test 12: failed WRDE_APPEND with WRDE_DOOFFS preserves we_wordc, the
> +   we_wordv pointer, the words and the leading NULL offset slots.  This
> +   exercises the we_offs arithmetic in the array duplication and in the
> +   error-path cleanup (we_wordv[we_offs + --we_wordc]).  */
> +static void
> +test_dooffs_append_error_preserves_state (void)
> +{
> +  printf ("info: test_dooffs_append_error_preserves_state\n");
> +  wordexp_t we = { 0 };
> +  we.we_offs = 3;
> +
> +  TEST_COMPARE (wordexp ("alpha beta", &we, WRDE_DOOFFS), 0);
> +  check_offs_null (&we);
> +  CHECK_WORDS (&we, "alpha", "beta");
> +
> +  char **saved_wordv = we.we_wordv;
> +  size_t saved_count = we.we_wordc;
> +  unsigned int saved_reallocs = relocating_reallocs;
> +
> +  /* "gamma" is a partial word added via w_addword (forcing a relocating
> +     realloc of we_wordv) before ')' triggers WRDE_BADCHAR.  */
> +  TEST_COMPARE (wordexp ("gamma )", &we, WRDE_APPEND | WRDE_DOOFFS),
> +		WRDE_BADCHAR);
> +  TEST_VERIFY (relocating_reallocs > saved_reallocs);
> +
> +  TEST_COMPARE (we.we_offs, 3);
> +  TEST_COMPARE (we.we_wordc, saved_count);
> +  TEST_VERIFY (we.we_wordv == saved_wordv);
> +  check_offs_null (&we);
> +  CHECK_WORDS (&we, "alpha", "beta");
> +  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
> +
> +  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 ();
> +  test_dooffs_append_success ();
> +  test_dooffs_append_error_preserves_state ();
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/posix/wordexp.c b/posix/wordexp.c
> index f0f69ee85d5..8fdc8b8caf0 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.
> @@ -2224,6 +2225,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 */
> @@ -2258,6 +2265,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.
> @@ -2338,7 +2362,7 @@ wordexp (const char *words, wordexp_t *pwordexp, int flags)
>  	    error = w_addword (pwordexp, NULL);
>  
>  	    if (error)
> -	      return error;
> +	      goto do_error;
>  	  }
>  
>  	break;
> @@ -2356,7 +2380,7 @@ wordexp (const char *words, wordexp_t *pwordexp, int flags)
>  	    error = w_addword (pwordexp, NULL);
>  
>  	    if (error)
> -	      return error;
> +	      goto do_error;
>  	  }
>  
>  	break;
> @@ -2422,10 +2446,18 @@ 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);
> +  /* There was no field separator at the end.  The only possible error
> +     from w_addword is WRDE_NOSPACE.  */
> +  error = w_addword (pwordexp, word);
> +  if (error != 0)
> +    goto do_error;
> +  free (saved_wordv);
> +  return 0;
>  
>  do_error:
>    /* Error:
> @@ -2436,11 +2468,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;
>  }


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368)
@ 2026-07-13 18:16 Adhemerval Zanella
  2026-07-13 18:18 ` Adhemerval Zanella Netto
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Adhemerval Zanella @ 2026-07-13 18:16 UTC (permalink / raw)
  To: libc-alpha; +Cc: DJ Delorie, Andreas K . Huettel

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).

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 posix/Makefile             |   1 +
 posix/tst-wordexp-append.c | 393 +++++++++++++++++++++++++++++++++++++
 posix/wordexp.c            |  69 ++++++-
 3 files changed, 454 insertions(+), 9 deletions(-)
 create mode 100644 posix/tst-wordexp-append.c

diff --git a/posix/Makefile b/posix/Makefile
index d9f897faa16..10d70ae0ae2 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -330,6 +330,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 00000000000..87f388f0a7e
--- /dev/null
+++ b/posix/tst-wordexp-append.c
@@ -0,0 +1,393 @@
+/* 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 <malloc.h>
+
+#include <support/check.h>
+#include <support/support.h>
+
+static unsigned int relocating_reallocs;
+
+/* w_addword grows we_wordv with realloc, make every call guaranteed to
+   relocate the block.  This makes BZ 34090 regression more deterministic.  */
+void *
+realloc (void *ptr, size_t size)
+{
+  if (ptr == NULL)
+    return malloc (size);
+  if (size == 0)
+    {
+      free (ptr);
+      return NULL;
+    }
+
+  void *new = malloc (size);
+  if (new == NULL)
+    return NULL;
+
+  /* Copy only what is valid in the old block to avoid reading past it.  */
+  size_t old = malloc_usable_size (ptr);
+  memcpy (new, ptr, old < size ? old : size);
+  /* Clobber the old block so that a stale we_wordv pointer restored on the
+     error path reads garbage instead of the old contents, which might
+     otherwise survive intact and mask the bug.  */
+  memset (ptr, 0x5a, old);
+  free (ptr);
+  relocating_reallocs++;
+  return new;
+}
+
+/* Verify that all words in we match the expected NULL-terminated
+   array.  */
+static void
+check_words (const wordexp_t *we, const char *const *expected)
+{
+  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_);				\
+  } 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;
+  unsigned int saved_reallocs = relocating_reallocs;
+
+  /* The interposed realloc guarantees the internal we_wordv buffer moves
+     during parsing, so the pointer-stability check below is meaningful.  */
+  TEST_COMPARE (wordexp ("append )", &we, WRDE_APPEND), WRDE_BADCHAR);
+  /* Verify that a relocating realloc actually happened during the failed
+     call, otherwise the pointer-stability check is vacuous.  */
+  TEST_VERIFY (relocating_reallocs > saved_reallocs);
+  TEST_COMPARE (we.we_wordc, saved_count);
+  TEST_VERIFY (we.we_wordv == saved_wordv);
+
+  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");
+
+  TEST_COMPARE (wordexp ("delta )", &we, WRDE_APPEND), WRDE_BADCHAR);
+
+  /* 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);
+
+  char **saved_wordv = we.we_wordv;
+
+  TEST_COMPARE (wordexp ("world", &we, WRDE_APPEND), 0);
+  TEST_COMPARE (we.we_wordc, 2);
+  /* A successful append works on a fresh copy of the array, so the
+     caller-visible pointer must have changed.  */
+  TEST_VERIFY (we.we_wordv != saved_wordv);
+  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");
+
+  TEST_COMPARE (wordexp ("bad |", &we, WRDE_APPEND), WRDE_BADCHAR);
+
+  /* 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;
+
+  /* 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);
+
+  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;
+
+  /* Unterminated double quote triggers WRDE_SYNTAX.  */
+  TEST_COMPARE (wordexp ("\"unterminated", &we, WRDE_APPEND), WRDE_SYNTAX);
+
+  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);
+}
+
+/* Verify that the leading we_offs slots are all NULL.  */
+static void
+check_offs_null (const wordexp_t *we)
+{
+  for (size_t i = 0; i < we->we_offs; i++)
+    TEST_VERIFY (we->we_wordv[i] == NULL);
+}
+
+/* Test 11: successful WRDE_APPEND with WRDE_DOOFFS and a non-zero we_offs.
+   The leading offset slots must stay NULL and words must land at
+   we_wordv[we_offs + i] across both the initial and the appended call.  */
+static void
+test_dooffs_append_success (void)
+{
+  printf ("info: test_dooffs_append_success\n");
+  wordexp_t we = { 0 };
+  we.we_offs = 2;
+
+  TEST_COMPARE (wordexp ("one two", &we, WRDE_DOOFFS), 0);
+  TEST_COMPARE (we.we_offs, 2);
+  check_offs_null (&we);
+  CHECK_WORDS (&we, "one", "two");
+
+  TEST_COMPARE (wordexp ("three", &we, WRDE_APPEND | WRDE_DOOFFS), 0);
+  TEST_COMPARE (we.we_offs, 2);
+  check_offs_null (&we);
+  CHECK_WORDS (&we, "one", "two", "three");
+  /* The NULL terminator must sit right after the last word.  */
+  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
+
+  wordfree (&we);
+}
+
+/* Test 12: failed WRDE_APPEND with WRDE_DOOFFS preserves we_wordc, the
+   we_wordv pointer, the words and the leading NULL offset slots.  This
+   exercises the we_offs arithmetic in the array duplication and in the
+   error-path cleanup (we_wordv[we_offs + --we_wordc]).  */
+static void
+test_dooffs_append_error_preserves_state (void)
+{
+  printf ("info: test_dooffs_append_error_preserves_state\n");
+  wordexp_t we = { 0 };
+  we.we_offs = 3;
+
+  TEST_COMPARE (wordexp ("alpha beta", &we, WRDE_DOOFFS), 0);
+  check_offs_null (&we);
+  CHECK_WORDS (&we, "alpha", "beta");
+
+  char **saved_wordv = we.we_wordv;
+  size_t saved_count = we.we_wordc;
+  unsigned int saved_reallocs = relocating_reallocs;
+
+  /* "gamma" is a partial word added via w_addword (forcing a relocating
+     realloc of we_wordv) before ')' triggers WRDE_BADCHAR.  */
+  TEST_COMPARE (wordexp ("gamma )", &we, WRDE_APPEND | WRDE_DOOFFS),
+		WRDE_BADCHAR);
+  TEST_VERIFY (relocating_reallocs > saved_reallocs);
+
+  TEST_COMPARE (we.we_offs, 3);
+  TEST_COMPARE (we.we_wordc, saved_count);
+  TEST_VERIFY (we.we_wordv == saved_wordv);
+  check_offs_null (&we);
+  CHECK_WORDS (&we, "alpha", "beta");
+  TEST_VERIFY (we.we_wordv[we.we_offs + we.we_wordc] == NULL);
+
+  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 ();
+  test_dooffs_append_success ();
+  test_dooffs_append_error_preserves_state ();
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/posix/wordexp.c b/posix/wordexp.c
index f0f69ee85d5..8fdc8b8caf0 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.
@@ -2224,6 +2225,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 */
@@ -2258,6 +2265,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.
@@ -2338,7 +2362,7 @@ wordexp (const char *words, wordexp_t *pwordexp, int flags)
 	    error = w_addword (pwordexp, NULL);
 
 	    if (error)
-	      return error;
+	      goto do_error;
 	  }
 
 	break;
@@ -2356,7 +2380,7 @@ wordexp (const char *words, wordexp_t *pwordexp, int flags)
 	    error = w_addword (pwordexp, NULL);
 
 	    if (error)
-	      return error;
+	      goto do_error;
 	  }
 
 	break;
@@ -2422,10 +2446,18 @@ 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);
+  /* There was no field separator at the end.  The only possible error
+     from w_addword is WRDE_NOSPACE.  */
+  error = w_addword (pwordexp, word);
+  if (error != 0)
+    goto do_error;
+  free (saved_wordv);
+  return 0;
 
 do_error:
   /* Error:
@@ -2436,11 +2468,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;
 }
-- 
2.43.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-12 14:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-01 18:51 [PATCH v2] posix: Fix wordexp WRDE_APPEND to preserve state on non-NOSPACE errors (BZ 34090, CVE-2026-6368) Adhemerval Zanella
2026-07-08 21:59 ` DJ Delorie
2026-07-13 18:11   ` Adhemerval Zanella Netto
2026-07-13 18:16 Adhemerval Zanella
2026-07-13 18:18 ` Adhemerval Zanella Netto
2026-07-13 18:53 ` DJ Delorie
2026-08-12 14:10 ` Andreas Schwab
2026-08-12 14:23   ` Adhemerval Zanella Netto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).