[v2] posix: Fix some crashes in wordexp [BZ #18096]

Message ID 20230322163957.9710-1-julian@cipht.net
State Committed
Commit 31bfe3ef4ea898df606cb6cc59ac72de27002b01
Headers
Series [v2] posix: Fix some crashes in wordexp [BZ #18096] |

Checks

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

Commit Message

Julian Squires March 22, 2023, 4:39 p.m. UTC
  Without these fixes, the first three included tests segfault (on a
NULL dereference); the fourth aborts on an assertion, which is itself
unnecessary.

Signed-off-by: Julian Squires <julian@cipht.net>
---
v2: removed the assert under seen_hash as requested.  Thanks for the
review.

 posix/wordexp-test.c |  4 ++++
 posix/wordexp.c      | 11 ++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)
  

Comments

Adhemerval Zanella March 28, 2023, 1:09 p.m. UTC | #1
On 22/03/23 13:39, Julian Squires via Libc-alpha wrote:
> Without these fixes, the first three included tests segfault (on a
> NULL dereference); the fourth aborts on an assertion, which is itself
> unnecessary.
> 
> Signed-off-by: Julian Squires <julian@cipht.net>

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
> v2: removed the assert under seen_hash as requested.  Thanks for the
> review.
> 
>  posix/wordexp-test.c |  4 ++++
>  posix/wordexp.c      | 11 ++++-------
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
> index f7a591149b..bae27d6cee 100644
> --- a/posix/wordexp-test.c
> +++ b/posix/wordexp-test.c
> @@ -117,6 +117,8 @@ struct test_case_struct
>      { 0, NULL, "$((010+0x10))", 0, 1, { "24" }, IFS },
>      { 0, NULL, "$((-010+0x10))", 0, 1, { "8" }, IFS },
>      { 0, NULL, "$((-0x10+010))", 0, 1, { "-8" }, IFS },
> +    { 0, NULL, "$(())", 0, 1, { "0", }, IFS },

Ok, empty arithmetic expansion.

> +    { 0, NULL, "$[]", 0, 1, { "0", }, IFS },
>

Ok, same as above (although with undocumented syntax not supported by all 
shells). 
>      /* Advanced parameter expansion */
>      { 0, NULL, "${var:-bar}", 0, 1, { "bar", }, IFS },
> @@ -138,6 +140,8 @@ struct test_case_struct
>      { 0, "12345", "${#var}", 0, 1, { "5", }, IFS },
>      { 0, NULL, "${var:-'}'}", 0, 1, { "}", }, IFS },
>      { 0, NULL, "${var-}", 0, 0, { NULL }, IFS },
> +    { 0, NULL, "${a?}", 0, 0, { NULL, }, IFS },
> +    { 0, NULL, "${#a=}", 0, 1, { "0", }, IFS },

Ok, parameter expansion.

>  
>      { 0, "pizza", "${var#${var}}", 0, 0, { NULL }, IFS },
>      { 0, "pepperoni", "${var%$(echo oni)}", 0, 1, { "pepper" }, IFS },
> diff --git a/posix/wordexp.c b/posix/wordexp.c
> index 0da98f5b08..b34c4a939b 100644
> --- a/posix/wordexp.c
> +++ b/posix/wordexp.c
> @@ -720,7 +720,7 @@ parse_arith (char **word, size_t *word_length, size_t *max_length,
>  	      ++(*offset);
>  
>  	      /* Go - evaluate. */
> -	      if (*expr && eval_expr (expr, &numresult) != 0)
> +	      if (expr && eval_expr (expr, &numresult) != 0)
>  		{
>  		  free (expr);
>  		  return WRDE_SYNTAX;
> @@ -758,7 +758,7 @@ parse_arith (char **word, size_t *word_length, size_t *max_length,
>  	      long int numresult = 0;
>  
>  	      /* Go - evaluate. */
> -	      if (*expr && eval_expr (expr, &numresult) != 0)
> +	      if (expr && eval_expr (expr, &numresult) != 0)
>  		{
>  		  free (expr);
>  		  return WRDE_SYNTAX;
> @@ -1790,7 +1790,7 @@ envsubst:
>  	    {
>  	      const char *str = pattern;
>  
> -	      if (str[0] == '\0')
> +	      if (!str || str[0] == '\0')
>  		str = _("parameter null or not set");
>  
>  	      __fxprintf (NULL, "%s: %s\n", env, str);
> @@ -1883,10 +1883,7 @@ envsubst:
>  			_itoa_word (value ? strlen (value) : 0,
>  				    &param_length[20], 10, 0));
>        if (free_value)
> -	{
> -	  assert (value != NULL);
> -	  free (value);
> -	}
> +	free (value);
>  
>        return *word ? 0 : WRDE_NOSPACE;
>      }

Ok.
  

Patch

diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
index f7a591149b..bae27d6cee 100644
--- a/posix/wordexp-test.c
+++ b/posix/wordexp-test.c
@@ -117,6 +117,8 @@  struct test_case_struct
     { 0, NULL, "$((010+0x10))", 0, 1, { "24" }, IFS },
     { 0, NULL, "$((-010+0x10))", 0, 1, { "8" }, IFS },
     { 0, NULL, "$((-0x10+010))", 0, 1, { "-8" }, IFS },
+    { 0, NULL, "$(())", 0, 1, { "0", }, IFS },
+    { 0, NULL, "$[]", 0, 1, { "0", }, IFS },
 
     /* Advanced parameter expansion */
     { 0, NULL, "${var:-bar}", 0, 1, { "bar", }, IFS },
@@ -138,6 +140,8 @@  struct test_case_struct
     { 0, "12345", "${#var}", 0, 1, { "5", }, IFS },
     { 0, NULL, "${var:-'}'}", 0, 1, { "}", }, IFS },
     { 0, NULL, "${var-}", 0, 0, { NULL }, IFS },
+    { 0, NULL, "${a?}", 0, 0, { NULL, }, IFS },
+    { 0, NULL, "${#a=}", 0, 1, { "0", }, IFS },
 
     { 0, "pizza", "${var#${var}}", 0, 0, { NULL }, IFS },
     { 0, "pepperoni", "${var%$(echo oni)}", 0, 1, { "pepper" }, IFS },
diff --git a/posix/wordexp.c b/posix/wordexp.c
index 0da98f5b08..b34c4a939b 100644
--- a/posix/wordexp.c
+++ b/posix/wordexp.c
@@ -720,7 +720,7 @@  parse_arith (char **word, size_t *word_length, size_t *max_length,
 	      ++(*offset);
 
 	      /* Go - evaluate. */
-	      if (*expr && eval_expr (expr, &numresult) != 0)
+	      if (expr && eval_expr (expr, &numresult) != 0)
 		{
 		  free (expr);
 		  return WRDE_SYNTAX;
@@ -758,7 +758,7 @@  parse_arith (char **word, size_t *word_length, size_t *max_length,
 	      long int numresult = 0;
 
 	      /* Go - evaluate. */
-	      if (*expr && eval_expr (expr, &numresult) != 0)
+	      if (expr && eval_expr (expr, &numresult) != 0)
 		{
 		  free (expr);
 		  return WRDE_SYNTAX;
@@ -1790,7 +1790,7 @@  envsubst:
 	    {
 	      const char *str = pattern;
 
-	      if (str[0] == '\0')
+	      if (!str || str[0] == '\0')
 		str = _("parameter null or not set");
 
 	      __fxprintf (NULL, "%s: %s\n", env, str);
@@ -1883,10 +1883,7 @@  envsubst:
 			_itoa_word (value ? strlen (value) : 0,
 				    &param_length[20], 10, 0));
       if (free_value)
-	{
-	  assert (value != NULL);
-	  free (value);
-	}
+	free (value);
 
       return *word ? 0 : WRDE_NOSPACE;
     }