Tags:
create new tag
, view all tags

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

TWiki version: TWikiRelease01Feb2003
TWiki plugins: DefaultPlugin, EmptyPlugin, InterwikiPlugin
Server OS: Red Hat Linux 6.2
Web server: Apache 1.3.6
Perl version: 5.005_03
Client OS:  
Web Browser:  

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

Edit | Attach | Watch | Print version | History: r5 < r4 < r3 < r2 < r1 | Backlinks | Raw View | Raw edit | More topic actions
Topic revision: r5 - 2004-08-20 - CrawfordCurrie
 
  • Learn about TWiki  
  • Download TWiki
This site is powered by the TWiki collaboration platform Powered by Perl Hosted by OICcam.com Ideas, requests, problems regarding TWiki? Send feedback. Ask community in the support forum.
Copyright © 1999-2015 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.