Bug 524588 (CVE-2009-4664)

Summary: CVE-2009-4664 fwbuilder: Insecure temporary file handling in the generated iptables script
Product: [Other] Security Response Reporter: Jan Lieskovsky <jlieskov>
Component: vulnerabilityAssignee: Red Hat Product Security <security-response-team>
Status: NEW --- QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: unspecifiedCC: j.golderer, redhat-bugzilla
Target Milestone: ---Keywords: Security
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://www.fwbuilder.org/docs/firewall_builder_release_notes.html#3.0.7
Whiteboard: impact=moderate,source=debian,reported=20090919,public=20090918,cvss2=4.4/AV:L/AC:M/Au:N/C:P/I:P/A:P
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:

Description Jan Lieskovsky 2009-09-21 07:23:08 EDT
An insecure temporary file handling in the generated iptables script was
found in fwbuilder. A local attacker could use this flaw to perform symlink attack against user running this script, which will result in overwrite of
arbitrary file writable by this script.

References:
-----------
http://www.fwbuilder.org/docs/firewall_builder_release_notes.html#3.0.7
Comment 1 Jan Lieskovsky 2009-09-21 07:24:21 EDT
This issue does NOT affect the latest versions of fwbuilder package, as 
shipped with Fedora release of 10 and 11.

This issue affects the version of fwbuilder package, as shipped with
Fedora Rawhide (fwbuilder-3.0.5-2.fc12).
Comment 2 Jan Lieskovsky 2009-09-21 07:33:47 EDT
While the upstream suggests the following changes in
/src/ipt/RoutingCompiler_ipt_writers.cpp:
RoutingCompiler_ipt::PrintRule::processNext() among versions 3.0.6:


154         compiler->output << "restore_script_output()" << endl;
155         compiler->output << "{" << endl;
156         compiler->output << "  exec 1>&3 2>&1" << endl;
157         compiler->output << "  cat /tmp/.fwbuilder.out" << endl;
158         compiler->output << "}" << endl;

184         compiler->output << "# redirect output to prevent ssh session from stalling"
185                          << endl;
186         compiler->output << "exec 3>&1" << endl;
187         compiler->output << "exec 1> /tmp/.fwbuilder.out" << endl;
188         compiler->output << "exec 2>&1" << endl;
189         compiler->output << endl;

and 3.0.7:


150         compiler->output << "TMPDIRNAME=\"/tmp/.fwbuilder.tempdir.$$\"" << endl;
151         compiler->output << "TMPFILENAME=\"$TMPDIRNAME/.fwbuilder.out\"" << endl;
152         compiler->output << "(umask 077 && mkdir $TMPDIRNAME) || exit 1" << endl;


159         compiler->output << "restore_script_output()" << endl;
160         compiler->output << "{" << endl;
161         compiler->output << "  exec 1>&3 2>&1" << endl;
162         compiler->output << "  cat $TMPFILENAME" << endl;
163         compiler->output << "  rm -rf $TMPDIRNAME" << endl;
164         compiler->output << "}" << endl;
165         compiler->output << endl;

The proposed patch doesn't seem to fix the issue (it shouldn't be
such a big problem for potential attacker to pre-generate 10^4 combinations
of symlinks for "/tmp/.fwbuilder.tempdir.$$/.fwbuilder.out" and "reuse"
the issue. More proper way how to address this seems to be to use:

mktemp -d -t .fwbuilder.tempdir.XXXXXX

  for safe temporary directory and 

mktemp -t .fwbuilder.out.XXXXXX

  for safe temporary file name.

  Mailed also Vadim Kurland regarding this doubt and if confirmed, we should patch Rawhide's fwbuilder with the newly proposed fix (this would need some care to include it into the code though).
Comment 3 Tomas Hoger 2009-12-21 09:20:10 EST
(In reply to comment #2)

> The proposed patch doesn't seem to fix the issue (it shouldn't be
> such a big problem for potential attacker to pre-generate 10^4 combinations
> of symlinks for "/tmp/.fwbuilder.tempdir.$$/.fwbuilder.out" and "reuse"
> the issue.

Aren't you overlooking this?

152         compiler->output << "(umask 077 && mkdir $TMPDIRNAME) || exit 1"

If someone pre-creates files/directories/links for all possible .fwbuilder.tempdir.$$, mkdir $TMPDIRNAME will exit with error and script will exit immediately too.  So easy to "DoS" the script, but not much beyond as far as I can see.

> More proper way how to address this seems to be to use:
> 
> mktemp -d -t .fwbuilder.tempdir.XXXXXX
> 
>   for safe temporary directory and 

Right, that's much harder to "DoS".

> mktemp -t .fwbuilder.out.XXXXXX
> 
>   for safe temporary file name.

This should not be needed when using temporary directory.

>   Mailed also Vadim Kurland regarding this doubt and if confirmed, we should
> patch Rawhide's fwbuilder with the newly proposed fix (this would need some
> care to include it into the code though).

I've you've got no reply, it might make sense to just port in upstream tracker.  It is possible upstream intentionally used temp dir as a fix instead of introducing mktemp.
Comment 4 Fedora Update System 2010-02-16 08:06:50 EST
fwbuilder-3.0.7-1.fc12, libfwbuilder-3.0.7-1.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 5 Vincent Danen 2010-03-03 16:34:11 EST
This has been assigned the name CVE-2009-4664.