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