Re: [Full-disclosure] Second-Order Symlink Vulnerabilities
coley@xxxxxxxxx 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.