Increasing TWiki's Robustness Against Shell Command Injection

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.)

• https://static.enyo.de/fw/patches/twiki/twiki-robustness-r3342.diff
• https://static.enyo.de/fw/patches/twiki/twiki-robustness-r3342.Changelog

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.

• For Developers: Increasing Robustness of Your Code
• TWiki Plugins
• Annoucements

For Developers: Increasing Robustness of Your Code

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.

TWiki Plugins

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.

  • https://static.enyo.de/fw/patches/twiki/imagegallery-robustness-20041128.diff

  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.

Annoucements

Security-related changes in these patches will be announced on the security-announce mailing list.

Revisions

• 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.

Florian Weimer
Home Blog (DE) Blog (EN) Impressum RSS Feeds