TWiki uses Perl's backtick operator in several places. The argument string of the backtick operator contains data from untrusted sources, causing potential security problems if shell metacharacters are involved. A patch to resolve these potential issues is provided.
In the past, several vulnerabilities involving shell command injection had to be fixed in TWiki (Metacharacters can be passed through to the shell in File Attach, TWiki Security Alert, 2003-10-19; TWiki search function allows arbitrary shell command execution
, TWiki Security Alert, 2004-11-13).
In order to correct the remaining potential issues and prevent further coding errors, the following patch introduces a replacement of Perl's backtick operator. This replacement constructs command line argument vectors in a safe way (by invoking the new subroutine TWiki::buildCommandLine
). Execution of arbitrary shell commands directly through shell metacharacters is no longer possible if TWiki::readFromProcess
or TWiki::readFromProcessArray
are used. (Indirect command injection is still possible if vulnerable programs and scripts are invoked through this interface, though.)
The current patch is relative to TWiki's Subversion trunk (http://ntwiki.ethermage.net:8181/svn/twiki/trunk), revision 3248. The Changelog file contains summaries of the changes. Please also read the README.robustness
file included in the patch, and keep in mind that you must edit your TWiki.cfg
file.
Note: This patch is known not to work with native Windows versions of TWiki.
With the following changes, your code can also benefit from the backtick replacement:
Look for code of the following form:
my $result = `$program $file1 $file2`;
Replace it with:
my $program = '/path/to/program %FILE1|F% %FILE2|F%'; [...] my ($result, $exit) = TWiki::readFromProcess ($program, FILE1 => $file1, FILE2 => $file2);
In this case, both $file1
and $file2
are files, so we use the F
flag in the $program
template.
Make sure that $program
is hard-coded in the source file or comes from TWiki.cfg
. Deriving it from topic content (for example, using TWiki's preferences system) is unsafe and dangerous!
Don't forget to check $exit
for errors (the value contains the shell exit status). Note that TWiki::readFromProcess
redirects standard error, so the output will include error messages.
If you use the backtick operator in the following form, you should use TWiki::readFromProcessArray
instead:
my @result = `$program $file1 $file2`;
Changes this code to:
my $program = '/path/to/program'; my $template = '%FILE1|F% %FILE2|F%'; [...] my @result = TWiki::readFromProcessArray ($program, $template, FILE1 => $file1, FILE2 => $file2);
As you can see, the argument lists differs slightly from TWiki::readFromProcess
: the program name is passed as a separate argument.
Note that TWiki::readFromProcessArray
does not redirect standard error. The caveat about $program
mentioned above also applies to TWiki::readFromProcessArray
, to both the $program
and $template
arguments.
Also look for unsafe uses of the qx//
operator (which is another form of the backtick operator).
Examine calls to Perl's open
function. If a subprocess is created, consider using the new safe pipes idiom of Perl 5.8 (see man perlipc
), or use TWiki::readFromProcess
or TWiki::readFromProcessArray
.
If open
is called to open a regular file, you might want to add a call to TWiki::normalizeFileName
. For example, change the following code:
my $handle; open $handle, "< $root/$file";
To:
my $handle; $file = TWiki::normalizeFileName ($file); open $handle, "< $file";
TWiki::normalizeFileName
will die if a directory traversal attempt is detected.
Replace calls to Perl's system
function with its safe variant (see the Perl documentation), or use TWiki::readFromProcess
(and add proper error checking).
There might be other operations which open files or spawn processes, and they might have unintuitive names. When using a particular Perl module, make sure that you do not allow TWiki users to pass data to them in dangerous ways. When in doubt, use TWiki.cfg
for storing configuration values, and not topics with preference settings.
Check for suspicious uses of eval
. Use the Safe
module if you cannot avoid mobile code. However, this is risky because Safe
probably has some security holes left on its own which might enable malicious TWiki users to escape from the sandbox.
Some TWiki plugins have been checked against the list above. The results are given below.
ImageGalleryPlugin
enables anyone who can create or edit image galleries to execute arbitrary shell commands on the web server hosting TWiki. The patch below addresses this security hole. It depends on the TWiki robustness patch described above.
You must add the following configuration settings to TWiki.cfg
before you apply the patch:
$ImageGalleryPlugin_CONVERT = '/usr/bin/convert -sample %RESIZE|N% %SOURCE|F% %TARGET|F%'; $ImageGalleryPlugin_IDENTIFY = '/usr/bin/identify %FILE|F%';
The Common Vulnerabilities and Exposures project has assigned the name CAN-2005-0516
to this issue.
ConditionalPlugin
uses the Safe
module for sandboxing. This means that if you install it, you should quickly apply Perl patches which address vulnerabilities in the Safe
module.
SpreadSheetPlugin
uses regular expressions to ensure that strings which are passed to eval
are harmless. While I could not discover an exploit, this approach should be considered poor engineering from a security standpoint.
Security-related changes in these patches will be announced on the security-announce mailing list.
2004-11-28: published
2004-12-15: The patch is diffed against a newer TWiki trunk version (r3342). Perl 5.6 support is no longer intentionally removed.
2005-02-25: Updated checklist to include eval
. Patch for ImageGalleryPlugin published.