Bug: Metacharacters can be passed through to the shell in File Attach

I noticed using a single-quote (') character in the comment field in Attach File causes the RCS checkin to fail. Seems shell characters aren't escaped or filtered in lib/TWiki/Store/RCSWrap.pm:_ci().

This means you can execute arbitrary shell commands as the Apache user by putting '; shell commands here' in the comment field. The checkin will fail, but the command is still executed.

Ideally _ci() and friends shouldn't use the shell at all - they should use the variant of the Perl system() function that accepts a list instead of a string, executing the first argument and passing the rest as command arguments without going through the shell.

This may be related to AttachCommentBadCharacters, but I think that has more to do with metacharacters in Wiki metadata than passing arguments to the RCS command.

Test case

Environment

-- MikeSmith - 19 Oct 2003

Follow up

Mike, thanks for reporting this. Indeed, this is a security issue!

Important: Administrators are encouraged to fix their TWiki installation as soon as possible!

Fix record

Is now fixed, patch available, in TWikiAlphaRelease and at TWiki.org.

Patch for file twiki/lib/TWiki/Store/RcsWrap.pm:

*** bu6/RcsWrap.pm      2003-01-04 17:37:22.000000000 -0800
--- RcsWrap.pm  2003-10-18 23:45:20.000000000 -0700
***************
*** 157,164 ****
--- 157,167 ----
      }
      $self->_saveFile( $self->file(), $text );
      $cmd = $self->{ciDateCmd};
+     $date =~ s/$TWiki::securityFilter//go;
      $cmd =~ s/%DATE%/$date/;
      $cmd =~ s/%USERNAME%/$user/;
+     $file =~ s/$TWiki::securityFilter//go;
+     $rcsFile =~ s/$TWiki::securityFilter//go;
      $cmd =~ s/%FILENAME%/$file $rcsFile/;
      $cmd =~ /(.*)/;
      $cmd = $1;       # safe, so untaint variable
***************
*** 383,390 ****
      my $cmd = $self->{"ciCmd"};
      my $rcsOutput = "";
      $cmd =~ s/%USERNAME%/$userName/;
      $cmd =~ s/%FILENAME%/$file/;
!     $comment = "none" if ( ! $comment );
      $cmd =~ s/%COMMENT%/$comment/;
      $cmd =~ /(.*)/;
      $cmd = $1;       # safe, so untaint variable
--- 386,395 ----
      my $cmd = $self->{"ciCmd"};
      my $rcsOutput = "";
      $cmd =~ s/%USERNAME%/$userName/;
+     $file =~ s/$TWiki::securityFilter//go;
      $cmd =~ s/%FILENAME%/$file/;
!     $comment = "none" unless( $comment );
!     $comment =~ s/[\"\'\`\;]//go;  # security, Codev.NoShellCharacterEscapingInFileAttachComment, MikeSmith
      $cmd =~ s/%COMMENT%/$comment/;
      $cmd =~ /(.*)/;
      $cmd = $1;       # safe, so untaint variable

Category: TWikiPatches

-- PeterThoeny - 19 Oct 2003