Re: [PATCH] var: ensure variables are fully initialised when unset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/11/2018 12:53, Ron Yorston wrote:
When a variable is unset by calling setvar(name, 0, 0) the code
to initialise the new, empty variable omits the trailing '='.

It's supposed to. A trailing = means the variable is set to an empty string. That's different from unset. You can see the difference with
set -u, or with ${var+set}. However, ...

Attempts to read the contents of the unset variable will result
in the uninitialised character at the end of the string being
accessed.

...this is indeed a bug which I've noticed as well. The code needs two trailing null bytes, not just one. Because of glibc internals, the out-of-bounds byte being read will almost certainly be zero on x86-64, but it's not a guarantee, and it could probably break more easily on other platforms.

It only affects shell-internal uses of variables, only for variables explicitly unset by a script (rather than unset by default), only for uses where the code does not explicitly check for unset beforehand. As far as scripts go, that just means PATH (as you found) I think, for interactive shells there are some more variables such as PS1/PS2/PS4/MAIL.

My patch is attached.

Cheers,
Harald van Dijk
diff --git a/src/var.c b/src/var.c
index 0d7e1db0..9163cca9 100644
--- a/src/var.c
+++ b/src/var.c
@@ -206,9 +206,9 @@ struct var *setvar(const char *name, const char *val, int flags)
 		vallen = strlen(val);
 	}
 	INTOFF;
-	p = mempcpy(nameeq = ckmalloc(namelen + vallen + 2), name, namelen);
+       p = mempcpy(nameeq = ckmalloc(namelen + vallen + 2), name, namelen + 1);
 	if (val) {
-		*p++ = '=';
+               p[-1] = '=';
 		p = mempcpy(p, val, vallen);
 	}
 	*p = '\0';

[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux