Forum OpenACS Development: Re: AOLserver Security Audit
Kriston,
I have discovered a potential security issue with AOLserver when used in
most recommended configurations.
There are two root causes of the vulnerability.
First, ns_getform creates new files in a temporary directory that is
system dependent. This directory is usually guaranteed to be world
writeable. This allows any local user to create a symbolic link to any
file on the system. Once this is done, AOLserver cannot under any
circumstance determine if the file is actually in the temp directory or
not.
The second issue is that most applications that process
multipart/form-data rely on varname.tmpfile to determine the location of
the temporary file. Nothing prevents an attacker from specifying 'any'
file on the filesystem. Some applications check the directory the file
is in and are only vulnerable to a local attacker.
The most vulnerable files are nsd.tcl, /etc/passwd and any ssl cert
files.
nsd.tcl can be easily protected by placing read-only access to the root
user, the other files cannot be protected in this way. However the ssl
cert files can be moved to an obscure location to make the attack less
efficient.
I have a proposed solution, which is accomplished by adding two new
configuration params:
ns_section "ns/server/${server}"
ns_param CreatMode 644
ns_param TmpDir "/web/stellar2/uploads/tmp"
ns_param TmpFileTemplate "nsgetformXXXXXX"
Oops, actually CreatMode doesn't work, but maybe should be added back
into the mix and used with the call to open.
I have patches for the 4.x series and the 3.x series of AOLserver.
$Header: /cvsroot/aolserver/aolserver/tcl/form.tcl,v 1.3 2000/08/01
20:36:17 kriston Exp $
I have the following patch (to ns_getform and another private proc):
115a116,117
<blockquote> set tmpdir [ns_config "ns/server/[ns_conn server]" TmpDir "/tmp"]
set tmpfiletemplate [ns_config "ns/server/[ns_conn server]" TmpFileTemplate
</blockquote>
"XXXXXX"]
117c119
< set tmpfile [ns_tmpnam]
---
<blockquote> set tmpfile [ns_mktemp ${tmpdir}/$tmpfiletemplate]
</blockquote>
199a202,203
<blockquote> set tmpdir [ns_config "ns/server/[ns_conn server]" TmpDir "/tmp"]
set tmpfiletemplate [ns_config "ns/server/[ns_conn server]" TmpFileTemplate
</blockquote>
"XXXXXX"]
201c205
< set tmpfile [ns_tmpnam]
---
<blockquote> set tmpfile [ns_mktemp ${tmpdir}/$tmpfiletemplate]
</blockquote>
for version 1.5 (latest available) of this file this patch works:
$Header: /cvsroot/aolserver/aolserver/tcl/form.tcl,v 1.5 2002/02/08
07:56:16 hobbs Exp $
102a103,104
<blockquote> set tmpdir [ns_config "ns/server/[ns_conn server]" TmpDir "/tmp"]
set tmpfiletemplate [ns_config "ns/server/[ns_conn server]" TmpFileTemplate
</blockquote>
"XXXXXX"]
104c106
< set tmpfile [ns_tmpnam]
---
<blockquote> set tmpfile [ns_mktemp ${tmpdir}/$tmpfiletemplate]
</blockquote>
A more complete solution would be to prevent users from passing in
variables ending in '.tmpfile'
I think the C procedures are in nsd/conn.c for the 3.x series and
currently in nsd/form.c, however there are many installation which will
not patch and recompile and they will need configuration
recommendations.
The configuration advice on the first vulnerability is to recommend
write access only to the user under which the webserver is running.
--Tom Jackson
and from Rob:
1. ns_mktemp and ns_tmpnam should be changed to use mkstemp.
2. The user cannot make nsd overwrite an existing file, because
_ns_parseformfp uses ns_openexcl.
3. AOLserver should follow standard Unix practice and use $TMP or
$TMPDIR as the temporary directory, rather than inventing another
config variable.
4. The sysadmin should set up a dedicated user id for each instance of
AOLserver. That user id should not own any files unless nsd
absolutely needs write access to the files. Obviously nsd doesn't
need write access to its own nsd.tcl
And my patch for the form.c file:
[tom@multi nsd]$ diff form.c form.old
210,216d209
< if (strstr(k, ".tmpfile")) {
< Ns_Log(Error,"querystring variable contains .tmpfile");
< Tcl_DStringFree(&kds);
< Tcl_DStringFree(&vds);
< return;
< }
< Ns_Log(Notice,"Found %s with value %s", k,
v);
Looks like the 3.x series has completely different code.
This could probably be tuned a little so .tmpfile must appear at the end
of the query key.