Re: [PATCH] var.c: check for valid variable name before printing in "export -p"

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxxx>]

On Tue, Feb 14, 2012 at 10:48:48AM +0000, harald@xxxxxxxxxx wrote:
> From: Harald Hoyer <harald@xxxxxxxxxx>
> 
> "export -p" prints all environment variables, without checking if the
> environment variable is a valid dash variable name.
> 
> IMHO, the only valid usecase for "export -p" is to eval the output.

Thanks a lot for the report and patch.

I'd prefer to fix this up at entry into the shell rather than
when we print out the variables.  So how about this patch?

commit 46d3c1a614f11f0d40a7e73376359618ff07abcd
Author: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date:   Sat Feb 25 15:35:18 2012 +0800

    [VAR] Sanitise environment variable names on entry
    
    On Tue, Feb 14, 2012 at 10:48:48AM +0000, harald@xxxxxxxxxx wrote:
    >
    > "export -p" prints all environment variables, without checking if the
    > environment variable is a valid dash variable name.
    >
    > IMHO, the only valid usecase for "export -p" is to eval the output.
    >
    > $ eval $(export -p); echo OK
    > OK
    >
    > Without this patch the following test does error out with:
    >
    > test.py:
    > import os
    > os.environ["test-test"]="test"
    > os.environ["test_test"]="test"
    > os.execv("./dash", [ './dash', '-c', 'eval $(export -p); echo OK' ])
    >
    > $ python test.py
    > ./dash: 1: export: test-test: bad variable name
    >
    > Of course the results can be more evil, if the environment variable
    > name is crafted, that it injects valid shell code.
    
    This patch fixes the issue by sanitising all environment variable names
    upon entry into the shell.
    
    Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

diff --git a/ChangeLog b/ChangeLog
index d1e84c9..8686332 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2012-02-25  Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
+
+	* Sanitise environment variable names on entry.
+
 2011-08-17  David S. Miller <davem@xxxxxxxxxxxxx>
 
 	* Allow building without LINEO support.
diff --git a/src/var.c b/src/var.c
index 027beff..dc90249 100644
--- a/src/var.c
+++ b/src/var.c
@@ -136,7 +136,8 @@ INIT {
 
 	initvar();
 	for (envp = environ ; *envp ; envp++) {
-		if (strchr(*envp, '=')) {
+		p = endofname(*envp);
+		if (p != *envp && *p == '=') {
 			setvareq(*envp, VEXPORT|VTEXTFIXED);
 		}
 	}

Cheers,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html