bugtraq - Re: Second-Order Symlink Vulnerabilities

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Tue, 07 Jun 2005 11:07:11 -0400
From: "Graham Reed" <greed@...ox.com>
To: coley@...re.org
Cc: full-disclosure@...ts.grok.org.uk, bugtraq@...urityfocus.com
Subject: Re: Second-Order Symlink Vulnerabilities

coley@...re.org writes:
> For everybuddy, we have: 
> 
>   258   g_snprintf(buf, 2048, "rm /tmp/.eb.%s.translator -f ; wget -O \
>      /tmp/.eb.%s.translator \
>   'http://world.altavista.com/sites/gben/pos/babelfish/tr?tt=urltext&lp=%s_%s&urltext=%s'",
>   259     getenv("USER"), getenv("USER"), from, to, string);
>   ...
>   ...
>   263   if(system(buf)!=0)  

Any program which removes a file by doing system("rm ...") has much, much 
worse problems than any kind of symlink attack. 

But the absolute WORST aspect of this example is the use of an environment 
variable to form a command line which is then dumped into the shell. 

The second worst thing is the use of the ";" to separate commands.  If you 
have batch code which does this, you almost always have bad code.  
Especially if the commands are run with user-provided input (and that 
includes EVERYTHING in the environment).  Even transient errors can cause 
"cd ${TARGET}; rm -rf *" to be a really, really, really bad idea.  Or the 
typical "cd ${SUBDIR}; make"--I've seen that blow the per-uesr process limit 
when SUBDIR doesn't exist, can't be accessed, or the fileserver is sulking. 

And it's unnecessary. 

int old_umask=umask(0600);
char tmpnam[]="/tmp/whateverXXXXXX";
int tmpfd=mkstemp(tmpnam);
char url[2048]; 

umask(old_umask);
if(snprintf(url,sizeof url,"http://.....",from,to,string)>=sizeof url)
{ abort(); }
fork()...execlp("wget","wget","-O",tmpnam,url,0);
wait()...
...use file...
unlink(tmpnam);
close(tmpfd); 

Or anything but a plain-vanilla "system()" with URLs and user-provided input 
in it at the same time. 

As far as I'm concerned, it is simply too difficult to predict all the 
possible quoting rules that may be required to safely invoke the shell via 
system().  So I don't.  And yes, that means I sometimes do a bit of 
fork/exec coding that system() saves me from.  Similary with redirecting to 
files--it's a little more work to do your own redirection (fork/open/exec), 
but you don't have to worry about some new shell putting meaning on 
something that you relied on. 

Actually, why don't we have "systeml", "systemlp", "systemv", and "systemvp" 
or something, so you get the fork/exec/wait done (and debugged) by the 
library, but you've got the reliability of argument passing of exec*? 

I think it's rather ironic that both your example problems seem to be in 
that annoying "check for new versions" thing that so many programs have 
added--each their own way. 

_______________________________________________
Full-Disclosure - We believe in it.
Charter: http://lists.grok.org.uk/full-disclosure-charter.html
Hosted and sponsored by Secunia - http://secunia.com/