<<< Date Index >>>     <<< Thread Index >>>

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.