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::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
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
$file2 are files, so we use the
F flag in the
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
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.
TWiki::readFromProcessArray does not redirect standard error. The caveat about
$program mentioned above also applies to
TWiki::readFromProcessArray, to both the
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
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";
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%';
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
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-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.