[PATCH] var: Do not add 1 to return value of strchrnul

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>]

наб <nabijaczleweli@xxxxxxxxxxxxxxxxxx> wrote:
>
> --- a/src/var.c
> +++ b/src/var.c
> @@ -266,7 +266,7 @@ struct var *setvareq(char *s, int flags)
>                        goto out;
> 
>                if (vp->func && (flags & VNOFUNC) == 0)
> -                       (*vp->func)(strchrnul(s, '=') + 1);
> +                       (*vp->func)(strchrnul(s, '=') + 1, flags);

Yes this was definitely broken.  strchrnul returns a pointer to the
final NUL character so adding one to it is just wrong.

However, I don't think we need to pass the flags to the action
function since none of them care about whether it's unset.  We
just need to pass a pointer to an empty string rather than some
bogus pointer.

---8<---
When a variable like OPTIND is unset dash may call the action
function with a bogus pointer because it tries to add one to
the return value of strchrnul unconditionally.

Use strchr and nullstr instead.

Link: https://bugs.debian.org/985478
Reported-by: наб <nabijaczleweli@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

diff --git a/src/var.c b/src/var.c
index ef9c2bd..f42bfd7 100644
--- a/src/var.c
+++ b/src/var.c
@@ -266,7 +266,7 @@ struct var *setvareq(char *s, int flags)
 			goto out;
 
 		if (vp->func && (flags & VNOFUNC) == 0)
-			(*vp->func)(strchrnul(s, '=') + 1);
+			(*vp->func)((strchr(s, '=') ?: nullstr - 1) + 1);
 
 		if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
 			ckfree(vp->text);
@@ -531,7 +531,8 @@ poplocalvars(void)
 			unsetvar(vp->text);
 		} else {
 			if (vp->func)
-				(*vp->func)(strchrnul(lvp->text, '=') + 1);
+				(*vp->func)((strchr(lvp->text, '=') ?:
+					     nullstr - 1) + 1);
 			if ((vp->flags & (VTEXTFIXED|VSTACK)) == 0)
 				ckfree(vp->text);
 			vp->flags = lvp->flags;
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt