From 21a9c71310be021ac65df61a7182da0318f5dee6 Mon Sep 17 00:00:00 2001 From: CrawfordCurrie Date: Sun, 21 Dec 2008 19:48:21 +0000 Subject: [PATCH] Item528: more analysis of validation and tainting issues, with many fixes git-svn-id: http://svn.foswiki.org/trunk@1490 0b4bb1d4-4e5a-0410-9cc4-b2b747904278 --- .../lib/Foswiki/Plugins/EditTablePlugin.pm | 4 +- .../lib/Foswiki/Plugins/InterwikiPlugin.pm | 7 +- .../lib/Foswiki/Plugins/PreferencesPlugin.pm | 1 + .../lib/Foswiki/Users/TopicUserMapping.pm | 3 + UnitTestContrib/test/unit/RcsTests.pm | 10 +- .../lib/Foswiki/Plugins/WysiwygPlugin.pm | 39 +++- core/lib/Assert.pm | 15 +- core/lib/Foswiki.pm | 127 +++++++------ core/lib/Foswiki.spec | 31 ++-- core/lib/Foswiki/Attrs.pm | 3 + core/lib/Foswiki/Compatibility.pm | 2 + core/lib/Foswiki/Engine/CGI.pm | 4 +- core/lib/Foswiki/Engine/CLI.pm | 8 +- core/lib/Foswiki/Form/ListFieldDefinition.pm | 2 +- core/lib/Foswiki/Form/Select.pm | 2 +- core/lib/Foswiki/Func.pm | 6 +- core/lib/Foswiki/LoginManager.pm | 16 +- .../lib/Foswiki/LoginManager/TemplateLogin.pm | 2 +- core/lib/Foswiki/Net/HTTPResponse.pm | 11 +- core/lib/Foswiki/OopsException.pm | 4 +- core/lib/Foswiki/Plugin.pm | 2 +- core/lib/Foswiki/Plugins.pm | 13 +- core/lib/Foswiki/Render.pm | 3 + core/lib/Foswiki/Sandbox.pm | 2 + core/lib/Foswiki/Store/RcsFile.pm | 81 ++++++--- core/lib/Foswiki/Store/RcsLite.pm | 17 +- core/lib/Foswiki/Store/RcsWrap.pm | 169 +++++++----------- .../Foswiki/Store/SearchAlgorithms/Forking.pm | 1 + core/lib/Foswiki/Templates.pm | 4 + core/lib/Foswiki/UI/Register.pm | 1 + core/lib/Foswiki/UI/Rest.pm | 11 +- core/lib/Foswiki/UI/View.pm | 2 + core/lib/Foswiki/Users/HtPasswdUser.pm | 2 + 33 files changed, 345 insertions(+), 260 deletions(-) diff --git a/EditTablePlugin/lib/Foswiki/Plugins/EditTablePlugin.pm b/EditTablePlugin/lib/Foswiki/Plugins/EditTablePlugin.pm index f179672ba7..60ad0e1fcb 100644 --- a/EditTablePlugin/lib/Foswiki/Plugins/EditTablePlugin.pm +++ b/EditTablePlugin/lib/Foswiki/Plugins/EditTablePlugin.pm @@ -76,14 +76,14 @@ sub initPlugin { } sub beforeCommonTagsHandler { - return unless $_[0] =~ /%EDIT(TABLE|CELL){(.*)}%/os; + return unless $_[0] =~ /%EDIT(TABLE|CELL){.*}%/s; require Foswiki::Plugins::EditTablePlugin::Core; Foswiki::Plugins::EditTablePlugin::Core::protectVariables( $_[0] ); } sub commonTagsHandler { - return unless $_[0] =~ /%EDIT(TABLE|CELL){(.*)}%/os; + return unless $_[0] =~ /%EDIT(TABLE|CELL){.*}%/s; addViewModeHeadersToHead(); require Foswiki::Plugins::EditTablePlugin::Core; diff --git a/InterwikiPlugin/lib/Foswiki/Plugins/InterwikiPlugin.pm b/InterwikiPlugin/lib/Foswiki/Plugins/InterwikiPlugin.pm index ef692cf4ff..67d93ae39b 100644 --- a/InterwikiPlugin/lib/Foswiki/Plugins/InterwikiPlugin.pm +++ b/InterwikiPlugin/lib/Foswiki/Plugins/InterwikiPlugin.pm @@ -50,10 +50,10 @@ use vars qw( %interSiteTable ); -# This should always be $Rev: 14913 (17 Sep 2007) $ so that TWiki can determine the checked-in +# This should always be $Rev$ so that TWiki can determine the checked-in # status of the plugin. It is used by the build automation tools, so # you should leave it alone. -$VERSION = '$Rev: 14913 (17 Sep 2007) $'; +$VERSION = '$Rev$'; # This is a free-form string you can use to "name" your own plugin version. # It is *not* used by the build automation tools, but is reported as part @@ -96,9 +96,6 @@ sub initPlugin { || 'InterWikis'; ( $interWeb, $interTopic ) = Foswiki::Func::normalizeWebTopicName( $interWeb, $interTopic ); - if( $interTopic =~ s/^(.*)\.// ) { - $interWeb = $1; - } my $text = Foswiki::Func::readTopicText( $interWeb, $interTopic, undef, 1 ); diff --git a/PreferencesPlugin/lib/Foswiki/Plugins/PreferencesPlugin.pm b/PreferencesPlugin/lib/Foswiki/Plugins/PreferencesPlugin.pm index 6c3724a9ff..f69bdaf4c0 100644 --- a/PreferencesPlugin/lib/Foswiki/Plugins/PreferencesPlugin.pm +++ b/PreferencesPlugin/lib/Foswiki/Plugins/PreferencesPlugin.pm @@ -123,6 +123,7 @@ sub beforeCommonTagsHandler { } elsif( $action eq 'save' ) { my( $meta, $text ) = Foswiki::Func::readTopic( $web, $topic ); + # SMELL: unchecked implicit untaint of value? $text =~ s(^((?:\t| )+\*\s(Set|Local)\s)(\w+)\s\=\s(.*)$) ($1._saveSet($query, $web, $topic, $3, $4, $formDef))mgeo; Foswiki::Func::saveTopic( $web, $topic, $meta, $text ); diff --git a/TopicUserMappingContrib/lib/Foswiki/Users/TopicUserMapping.pm b/TopicUserMappingContrib/lib/Foswiki/Users/TopicUserMapping.pm index 6673c761e9..85bfe32451 100755 --- a/TopicUserMappingContrib/lib/Foswiki/Users/TopicUserMapping.pm +++ b/TopicUserMappingContrib/lib/Foswiki/Users/TopicUserMapping.pm @@ -378,6 +378,8 @@ sub addUser { $web = $1 || $Foswiki::cfg{UsersWebName}; $name = $2; $odate = $3; + # Filter-in date format dd Mmm yyyy + $odate = '' unless $odate =~ /^\d+\s+[A-Za-z]+\s+\d+$/; $insidelist = 1; } elsif ( $line =~ /^\s+\*\s([A-Z]) - / ) { @@ -863,6 +865,7 @@ sub mapper_getEmails { # Now try the topic text foreach my $l ( split( /\r?\n/, $text ) ) { if ( $l =~ /^\s+\*\s+E-?mail:\s*(.*)$/mi ) { + # SMELL: implicit unvalidated untaint push @addresses, split( /;/, $1 ); } } diff --git a/UnitTestContrib/test/unit/RcsTests.pm b/UnitTestContrib/test/unit/RcsTests.pm index 581edd3b31..62aa76b01a 100644 --- a/UnitTestContrib/test/unit/RcsTests.pm +++ b/UnitTestContrib/test/unit/RcsTests.pm @@ -214,8 +214,16 @@ sub verify_simple9 { $this->checkGetRevision([ "", "\n", "\n\n", "a", "a\n", "a\n\n", "\na","\n\na", "" ]); } -sub verify_simple10 { +# coMustCopy should only affect RcsWrap, but test them both anyway +sub verify_simple10a { my $this = shift; + $Fowiki::cfg{RCS}{coMustCopy} = 0; + $this->checkGetRevision([ "a", "b", "a\n", "b", "a", "b\n","a\nb\n", "a\nc\n" ]); +} + +sub verify_simple10b { + my $this = shift; + $Fowiki::cfg{RCS}{coMustCopy} = 1; $this->checkGetRevision([ "a", "b", "a\n", "b", "a", "b\n","a\nb\n", "a\nc\n" ]); } diff --git a/WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin.pm b/WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin.pm index e02ad12e79..e9a16cedb9 100644 --- a/WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin.pm +++ b/WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin.pm @@ -39,7 +39,7 @@ use vars qw( %TWikiCompatibility @refs ); $SHORTDESCRIPTION = 'Translator framework for Wysiwyg editors'; $NO_PREFS_IN_TOPIC = 1; -$VERSION = '$Rev: 15843 $'; +$VERSION = '$Rev$'; $RELEASE = '03 Aug 2008'; @@ -732,14 +732,29 @@ sub _restHTML2TML { return undef; # to prevent further processing } +# SMELL: foswiki supports proper REST usage of the upload script, +# so debatable if this is required any more sub _restUpload { my ($session, $plugin, $verb, $response) = @_; my $query = Foswiki::Func::getCgiQuery(); - my $topic = $query->param('topic'); - $topic =~ /^(.*)\.([^.]*)$/; - my $web = $1; - $topic = $2; - + my ($web, $topic) = Foswiki::Func::normalizeWebTopicName( undef, + $query->param('topic')); + $web = Foswiki::Sandbox::untaint( + $web, + sub { + return $web if Foswiki::Func::isValidWebName($web, 1); + return undef; + }); + $topic = Foswiki::Sandbox::untaint( + $topic, + sub { + return $topic if Foswiki::Func::isValidTopicName($topic, 1); + return undef; + }); + unless (defined $web && defined $topic) { + returnRESTResult($response, 401, "Access denied"); + return undef; # to prevent further processing + } my $hideFile = $query->param('hidefile') || ''; my $fileComment = $query->param('filecomment') || ''; my $createLink = $query->param('createlink') || ''; @@ -838,6 +853,18 @@ sub _restAttachments { my ($session, $plugin, $verb, $response) = @_; my ($web, $topic) = Foswiki::Func::normalizeWebTopicName( undef, Foswiki::Func::getCgiQuery()->param('topic')); + $web = Foswiki::Sandbox::untaint( + $web, + sub { + return $web if Foswiki::Func::isValidWebName($web, 1); + return undef; + }); + $topic = Foswiki::Sandbox::untaint( + $topic, + sub { + return $topic if Foswiki::Func::isValidTopicName($topic, 1); + return undef; + }); my ($meta, $text) = Foswiki::Func::readTopic($web, $topic); unless (Foswiki::Func::checkAccessPermission( 'VIEW', Foswiki::Func::getWikiName(), $text, $topic, $web, $meta)) { diff --git a/core/lib/Assert.pm b/core/lib/Assert.pm index 20db6d3126..00b52bbfec 100644 --- a/core/lib/Assert.pm +++ b/core/lib/Assert.pm @@ -10,7 +10,7 @@ require 5.006; # 1. ASSERT instead of assert # 2. has to be _explicitly enabled_ using the $ENV{ASSERT} # 3. should and shouldnt have been removed -# 4. Added UNTAINTED +# 4. Added UNTAINTED and TAINT # # Usage is as for Carp::Assert except that you have to explicitly # enable asserts using the environment variable ENV{FOSWIKI_ASSERTS} @@ -19,12 +19,13 @@ require 5.006; use strict; -use vars qw(@ISA $VERSION %EXPORT_TAGS); +use vars qw(@ISA $VERSION %EXPORT_TAGS $DIRTY); BEGIN { $VERSION = '0.01'; + $DIRTY = $ENV{PATH}; # Used in TAINT - $EXPORT_TAGS{NDEBUG} = [qw(ASSERT UNTAINTED DEBUG)]; + $EXPORT_TAGS{NDEBUG} = [qw(ASSERT UNTAINTED TAINT DEBUG)]; $EXPORT_TAGS{DEBUG} = $EXPORT_TAGS{NDEBUG}; Exporter::export_tags(qw(NDEBUG DEBUG)); } @@ -47,7 +48,7 @@ sub import { else { my $caller = caller; *{ $caller . '::ASSERT' } = \&noop; - *{ $caller . '::UNTAINTED' } = \&ASSERTS_OFF; + *{ $caller . '::TAINT' } = sub { $_[0] }; *{ $caller . '::DEBUG' } = \&ASSERTS_OFF; } use strict 'refs'; @@ -65,10 +66,16 @@ sub ASSERT ($;$) { return undef; } +# Test if a value is untainted sub UNTAINTED($) { local ( @_, $@, $^W ) = @_; my $x; return eval { $x = $_[0], kill 0; 1 }; } +# Taint the datum passed and return the tainted value +sub TAINT($) { + return substr($_[0].$DIRTY, 0, length($_[0])); +} + 1; diff --git a/core/lib/Foswiki.pm b/core/lib/Foswiki.pm index 7d9dea4ca2..ea4efae4cc 100644 --- a/core/lib/Foswiki.pm +++ b/core/lib/Foswiki.pm @@ -101,25 +101,26 @@ sub _getLibDir { "WARNING: Foswiki lib path $foswikiLibDir is relative; you should make it absolute, otherwise some scripts may not run from the command line."; my $bin; - # TSA SMELL : Should not assume environment variables and get data from request + # SMELL : Should not assume environment variables; get data from request if ( $ENV{SCRIPT_FILENAME} - && $ENV{SCRIPT_FILENAME} =~ /^(.+)\/[^\/]+$/ ) + && $ENV{SCRIPT_FILENAME} =~ m#^(.+)/.+?$# ) { # CGI script name + # implicit untaint OK, because of use of $SCRIPT_FILENAME $bin = $1; } - elsif ( $0 =~ /^(.*)\/.*?$/ ) { + elsif ( $0 =~ m#^(.*)/.*?$# ) { # program name + # implicit untaint OK, because of use of $PROGRAM_NAME ($0) $bin = $1; } else { # last ditch; relative to current directory. require Cwd; - import Cwd qw( cwd ); - $bin = cwd(); + $bin = Cwd::cwd(); } $foswikiLibDir = "$bin/$foswikiLibDir/"; @@ -261,15 +262,15 @@ BEGIN { $Foswiki::cfg{OS} = 'OS2'; } -# Validate and untaint Apache's SERVER_NAME Environment variable -# for use in referencing virtualhost-based paths for separate data/ and templates/ instances, etc + # Validate and untaint Apache's SERVER_NAME Environment variable + # for use in referencing virtualhost-based paths for separate data/ + # and templates/ instances, etc $ENV{SERVER_NAME} = Foswiki::Sandbox::untaint( $ENV{SERVER_NAME}, sub { - my $sn = shift; - return undef unless defined $sn; - return $sn if $sn =~ + return undef unless defined $ENV{SERVER_NAME}; + return $ENV{SERVER_NAME} if $ENV{SERVER_NAME} =~ /^(([a-z0-9]([a-z0-9\-]{0,61}[a-z0-9])?\.)+[a-z]{2,6})$/i; return undef; }); @@ -710,6 +711,7 @@ sub generateHTTPHeaders { || ''; if ($pluginHeaders) { foreach ( split /\r?\n/, $pluginHeaders ) { + # Implicit untaint OK; data from plugin handler if (m/^([\-a-z]+): (.*)$/i) { $hopts->{$1} = $2; } @@ -741,27 +743,26 @@ sub generateHTTPHeaders { sub _isRedirectSafe { my $redirect = shift; - #TODO: this should really use URI - if ( ( !$Foswiki::cfg{AllowRedirectUrl} ) - && ( $redirect =~ m!^([^:]*://[^/]*)/*(.*)?$! ) ) - { - my $host = $1; + return 1 if ( $Foswiki::cfg{AllowRedirectUrl} ); - #remove trailing /'s to match - $Foswiki::cfg{DefaultUrlHost} =~ m!^([^:]*://[^/]*)/*(.*)?$!; - my $expected = $1; - - if ( defined( $Foswiki::cfg{PermittedRedirectHostUrls} ) - && $Foswiki::cfg{PermittedRedirectHostUrls} ne '' ) - { - my @permitted = - map { s!^([^:]*://[^/]*)/*(.*)?$!$1!; $1 } - split( /,\s*/, $Foswiki::cfg{PermittedRedirectHostUrls} ); - return 1 if ( grep ( { uc($host) eq uc($_) } @permitted ) ); + #TODO: this should really use URI + # Compare protocol, host name and port number + if ( $redirect =~ m!^(.*?://[^/]*)! ) { + # implicit untaints OK because result not used + my $target = uc( $1 ); + + $Foswiki::cfg{DefaultUrlHost} =~ m!^(.*?://[^/]*)!; + return 1 if ( $target eq uc($1) ); + + if ( $Foswiki::cfg{PermittedRedirectHostUrls} ) { + foreach my $red (split( + /\s*,\s*/, $Foswiki::cfg{PermittedRedirectHostUrls} )) { + $red =~ m!^(.*?://[^/]*)!; + return 1 if ( $target eq uc($1) ); + } } - return ( uc($host) eq uc($expected) ); } - return 1; + return 0; } =begin TML @@ -803,7 +804,7 @@ sub redirectto { $this->normalizeWebTopicName( $this->{webName}, $redirecturl ); # capture anchor - my ($topic, $anchor) = $t =~ m/^(.*)#(.*)$/; + my ($topic, $anchor) = split('#', $t, 2); $t = $topic if $topic; my @attrs = (); push (@attrs, '#'=>$anchor) if $anchor; @@ -852,7 +853,7 @@ sub redirect { if ( $passthru && defined $query->method() ) { my $existing = ''; if ( $url =~ s/\?(.*)$// ) { - $existing = $1; + $existing = $1; # implicit untaint OK; recombined later } if ( $query->method() eq 'POST' ) { @@ -1262,8 +1263,8 @@ the web defaults to $Foswiki::cfg{UsersWebName}. If there is no topic specification, or the topic is '0', the topic defaults to the web home topic name. -*WARNING* this function untaints the web and topic names, but does not check -the validity of same. +*WARNING* if the input topic name is tainted, then the output web and +topic names will be tainted. =cut @@ -1275,15 +1276,22 @@ sub normalizeWebTopicName { if ( $topic =~ m|^(.*)[./](.*?)$| ) { $web = $1; $topic = $2; + + if ( DEBUG && !UNTAINTED($_[2] )) { + # retaint data untainted by RE above + $web = TAINT($web); + $topic = TAINT($topic); + } } $web ||= $cfg{UsersWebName}; $topic ||= $cfg{HomeTopicName}; - while ( $web =~ -s/%((MAIN|TWIKI|USERS|SYSTEM|DOC)WEB)%/_expandTagOnTopicRendering( $this,$1)||''/e - ) - { + # MAINWEB and TWIKIWEB expanded for compatibility reasons + while ( $web =~ s/%((MAIN|TWIKI|USERS|SYSTEM|DOC)WEB)%/ + $this->_expandTagOnTopicRendering( $1 ) || ''/e ) { } - $web =~ s#\.#/#go; + # Normalize web name to use / and not . as a subweb separator + $web =~ s#\.#/#g; + return ( $web, $topic ); } @@ -1336,18 +1344,18 @@ sub new { require Foswiki::Store; $this->{store} = new Foswiki::Store($this); - $this->{remoteUser} = - $login; #use login as a default (set when running from cmd line) + # use login as a default (set when running from cmd line) + $this->{remoteUser} = $login; + require Foswiki::Users; $this->{users} = new Foswiki::Users($this); $this->{remoteUser} = $this->{users}->{remoteUser}; - # Make %ENV safer, preventing hijack of the search path - # SMELL: can this be done in a BEGIN block? Or is the environment - # set per-query? - # TWikibug:Item4382: Default $ENV{PATH} must be untainted because Foswiki runs - # with use strict and calling external programs that writes on the disk - # will fail unless Perl seens it as set to safe value. + # Make %ENV safer, preventing hijack of the search path. The + # environment is set per-query, so this can't be done in a BEGIN. + # TWikibug:Item4382: Default $ENV{PATH} must be untainted because + # Foswiki runs with use strict and calling external programs that + # writes on the disk will fail unless Perl seens it as set to safe value. if ( $Foswiki::cfg{SafeEnvPath} ) { $ENV{PATH} = $Foswiki::cfg{SafeEnvPath}; } @@ -1384,6 +1392,7 @@ sub new { # SMELL: this is a really dangerous hack. It will fail # spectacularly with mod_perl. # SMELL: why not just use $query->script_name? + # SMELL: unchecked implicit untaint? $this->{scriptUrlPath} = $1; } @@ -1399,13 +1408,13 @@ sub new { $this->redirect($topic); return $this; } - elsif ( $topic =~ /((?:.*[\.\/])+)(.*)/ ) { + elsif ( $topic =~ m#^(.*)[./](.*?)$# ) { - # is 'bin/script?topic=Webname.SomeTopic' + # is '?topic=Webname.SomeTopic' + # implicit untaint OK - validated later $web = $1; $topic = $2; - $web =~ s/\./\//go; - $web =~ s/\/$//o; + $web =~ s/\./\//g; # jump to WebHome if 'bin/script?topic=Webname.' $topic = $Foswiki::cfg{HomeTopicName} if ( $web && !$topic ); @@ -1418,21 +1427,22 @@ sub new { } my $pathInfo = $query->path_info(); - $pathInfo =~ s|//*|/|g; #multiple //'s are illogical - $pathInfo =~ s|/$||; #trailing / does not mean WebHome + $pathInfo =~ s|//+|/|g; # multiple //'s are illogical + $pathInfo =~ s#/+$##; # Get the web and topic names from PATH_INFO - if ( $pathInfo =~ /\/((?:.*[\.\/])+)(.*)/ ) { + if ( $pathInfo =~ m#^/(.*)[./](.*?)$# ) { - # is 'bin/script/Webname/SomeTopic' or 'bin/script/Webname/' + # is '/Webname/SomeTopic' or '/Webname' + # implicit untaint OK - validated later $web = $1 unless $web; $topic = $2 unless $topic; - $web =~ s/\./\//go; - $web =~ s/\/$//o; + $web =~ s/\./\//g; } - elsif ( $pathInfo =~ /\/(.*)/ ) { + elsif ( $pathInfo =~ m#^/(.*?)$# ) { # is 'bin/script/Webname' or 'bin/script/' + # implicit untaint OK - validated later $web = $1 unless $web; } @@ -2168,6 +2178,7 @@ sub parseSections { foreach my $bit ( split( /(%(?:START|END)SECTION(?:{.*?})?%)/, $_[0] ) ) { if ( $bit =~ /^%STARTSECTION(?:{(.*)})?%$/ ) { require Foswiki::Attrs; + # SMELL: unchecked implicit untaint? my $attrs = new Foswiki::Attrs($1); $attrs->{type} ||= 'section'; $attrs->{name} = @@ -2198,6 +2209,7 @@ sub parseSections { } elsif ( $bit =~ /^%ENDSECTION(?:{(.*)})?%$/ ) { require Foswiki::Attrs; + # SMELL: unchecked implicit untaint? my $attrs = new Foswiki::Attrs($1); $attrs->{type} ||= 'section'; $attrs->{name} = $attrs->{_DEFAULT} || $attrs->{name} || ''; @@ -2672,6 +2684,7 @@ sub _processTags { # /s so you can have newlines in parameters if ( $stackTop =~ m/^%(($regex{tagNameRegex})(?:{(.*)})?)$/so ) { + # SMELL: unchecked implicit untaint? my ( $expr, $tag, $args ) = ( $1, $2, $3 ); #print STDERR ' ' x $tell,"POP $tag\n"; @@ -3711,7 +3724,7 @@ sub ENV { && defined $Foswiki::cfg{AccessibleENV} && $key =~ /$Foswiki::cfg{AccessibleENV}/o; my $val; - if ( $key =~ /^HTTPS?_(.*)/ ) { + if ( $key =~ /^HTTPS?_(\w+)/ ) { $val = $this->{request}->header($1); } elsif ( $key eq 'REQUEST_METHOD' ) { diff --git a/core/lib/Foswiki.spec b/core/lib/Foswiki.spec index bd49ad1d11..214947e0aa 100644 --- a/core/lib/Foswiki.spec +++ b/core/lib/Foswiki.spec @@ -86,17 +86,6 @@ my $OS = $Foswiki::cfg{OS} || ''; # This is the root of all Foswiki URLs e.g. http://myhost.com:123. # $Foswiki::cfg{DefaultUrlHost} = 'http://your.domain.com'; -# **STRING** -# If your host has aliases (such as both www.foswiki.org and foswiki.org, and some IP addresses) -# you need to list them to tell Foswiki that redirecting to them is OK. Foswiki uses redirection -# as part of its normal mode of operation when it changes between editing and viewing. -# The security setting {AllowRedirectUrl} is per default disabled making redirecting to other -# domains restricted to prevent Foswiki from being used in phishing attacks to protect it from -# middleman exploits. You can add additional URLs to this setting to enable redirects to -# additional trusted sites. Enter as comma separated list of URLs or hostnames. The URL must -# be in the format http://your.domain.com. -$Foswiki::cfg{PermittedRedirectHostUrls} = ''; - # **PATH M** # This is the 'cgi-bin' part of URLs used to access the Foswiki bin # directory e.g. /foswiki/bin
@@ -534,11 +523,25 @@ $Foswiki::cfg{RemovePortNumber} = $FALSE; # public sites should not enable it. Note: It is possible to # redirect to a topic regardless of this setting, such as # topic=OtherTopic or redirectto=Web.OtherTopic. -# To enable redirection to a just list of trusted URLs keep this setting -# disabled and add a list of trusted URL to the {PermittedRedirectHostUrls} -# setting in the General path settings section. +# To enable redirection to a list of trusted URLs, keep this setting +# disabled and set the {PermittedRedirectHostUrls}. $Foswiki::cfg{AllowRedirectUrl} = $FALSE; +# **STRING EXPERT** +# If your host has aliases (such as both www.foswiki.org and foswiki.org +# and some IP addresses) you need to tell Foswiki that redirecting to them +# is OK. Foswiki uses redirection as part of its normal mode of operation +# when it changes between editing and viewing. +# To prevent Foswiki from being used in phishing attacks and to protect it +# from middleman exploits, the security setting {AllowRedirectUrl} is by +# default disabled, restricting redirection to other domains. If a redirection +# to a different host is attempted, the target URL is compared against this +# list of additional trusted sites, and only if it matches is the redirect +# permitted.
+# Enter as a comma separated list of URLs (protocol, hostname and (optional) +# port) e.g. http://your.domain.com:8080,https://other.domain.com +$Foswiki::cfg{PermittedRedirectHostUrls} = ''; + # **REGEX EXPERT** # Defines the filter-in regexp that must match the names of environment # variables that can be seen using the %ENV{}% Foswiki variable. Set it to diff --git a/core/lib/Foswiki/Attrs.pm b/core/lib/Foswiki/Attrs.pm index 224ee910df..47c5daf6b8 100644 --- a/core/lib/Foswiki/Attrs.pm +++ b/core/lib/Foswiki/Attrs.pm @@ -126,6 +126,7 @@ sub new { # otherwise the whole string - sans padding - is the default else { + # SMELL: unchecked implicit untaint? if ( $string =~ m/^\s*(.*?)\s*$/s && !defined( $this->{$DEFAULTKEY} ) ) { @@ -134,6 +135,7 @@ sub new { last; } } + # SMELL: unchecked implicit untaint? elsif ( $string =~ m/^\s*(.*?)\s*$/s ) { $this->{$DEFAULTKEY} = $1 if ($first); last; @@ -233,6 +235,7 @@ sub extractValue { else { # test if format: { "value" ... } + # SMELL: unchecked implicit untaint? if ( $str =~ /(^|\=\s*\"[^\"]*\")\s*\"(.*?)\"\s*(\w+\s*=\s*\"|$)/ ) { # is: %VAR{ "value" }% diff --git a/core/lib/Foswiki/Compatibility.pm b/core/lib/Foswiki/Compatibility.pm index 206dda108a..d7d6c75104 100644 --- a/core/lib/Foswiki/Compatibility.pm +++ b/core/lib/Foswiki/Compatibility.pm @@ -90,6 +90,7 @@ sub _upgradeCategoryItem { $scatname = $catname; #$scatname =~ s/[^a-zA-Z0-9]//g; + # SMELL: unchecked implicit untaint? $src =~ /(.*)/; if ($1) { $src = $1; @@ -219,6 +220,7 @@ sub _getOldAttachAttr { ( $before, $filePath, $after ) = split( /<(?:\/)*TwkFilePath>/, $atext ); if ( !$filePath ) { $filePath = ''; } + # SMELL: unchecked implicit untaint $filePath =~ s///go; if ($1) { $filePath = $1; } else { $filePath = ''; } diff --git a/core/lib/Foswiki/Engine/CGI.pm b/core/lib/Foswiki/Engine/CGI.pm index 6721d842d7..750004b968 100644 --- a/core/lib/Foswiki/Engine/CGI.pm +++ b/core/lib/Foswiki/Engine/CGI.pm @@ -110,7 +110,7 @@ sub preparePath { # This handles twiki_cgi; use the first path el after the script # name as the function $pathInfo =~ m{^/([^/]+)(.*)}; - my $first = $1 || ''; + my $first = $1; # implicit untaint OK; checked below if ( exists $Foswiki::cfg{SwitchBoard}{$first} ) { # The path is of the form script/function/... @@ -133,7 +133,7 @@ sub prepareBody { my $cgi = new CGI(); my $err = $cgi->cgi_error; throw Foswiki::EngineException( $1, $2 ) - if defined $err && $err =~ /\s*(\d{3})\s*(.*)/o; + if defined $err && $err =~ /\s*(\d{3})\s*(.*)/; $this->{cgi} = $cgi; } diff --git a/core/lib/Foswiki/Engine/CLI.pm b/core/lib/Foswiki/Engine/CLI.pm index 33f12662e3..08e24f3295 100644 --- a/core/lib/Foswiki/Engine/CLI.pm +++ b/core/lib/Foswiki/Engine/CLI.pm @@ -12,9 +12,11 @@ Refer to Foswiki::Engine documentation for explanation about methos below. =cut package Foswiki::Engine::CLI; +use base 'Foswiki::Engine'; use strict; -use base 'Foswiki::Engine'; +use Assert; + use Foswiki::Request; use Foswiki::Request::Upload; use Foswiki::Response; @@ -26,9 +28,9 @@ sub run { my $name; my $arg = shift @args; if ( $arg =~ /^-([a-z0-9_]+)/) { - ($name, $arg) = ($1, shift( @args )); + ($name, $arg) = (TAINT($1), shift( @args )); } elsif ( $arg =~ /([a-z0-9_]+)=(.*)$/i ) { - ($name, $arg) = ($1, $2); + ($name, $arg) = (TAINT($1), TAINT($2)); } if ($name && $name eq 'user' ) { $this->{user} = $arg; diff --git a/core/lib/Foswiki/Form/ListFieldDefinition.pm b/core/lib/Foswiki/Form/ListFieldDefinition.pm index a768f69727..e4f82608ad 100644 --- a/core/lib/Foswiki/Form/ListFieldDefinition.pm +++ b/core/lib/Foswiki/Form/ListFieldDefinition.pm @@ -66,7 +66,7 @@ sub getOptions { $inBlock = 1; } elsif (/^\s*\|\s*([^|]*?)\s*\|/) { - push( @vals, $1 ) if ($inBlock); + push( @vals, TAINT($1) ) if ($inBlock); } else { $inBlock = 0; diff --git a/core/lib/Foswiki/Form/Select.pm b/core/lib/Foswiki/Form/Select.pm index cf0b945439..1239fb1ce8 100644 --- a/core/lib/Foswiki/Form/Select.pm +++ b/core/lib/Foswiki/Form/Select.pm @@ -52,7 +52,7 @@ sub getOptions { my $str; foreach my $val (@$vals) { if ( $val =~ /^(.*?[^\\])=(.*)$/ ) { - $str = $1; + $str = TAINT($1); $val = $2; $str =~ s/\\=/=/g; } diff --git a/core/lib/Foswiki/Func.pm b/core/lib/Foswiki/Func.pm index 6053c79294..ef37772760 100644 --- a/core/lib/Foswiki/Func.pm +++ b/core/lib/Foswiki/Func.pm @@ -2459,7 +2459,7 @@ sub isValidWikiWord { =begin TML ----+++ isValidWebName( $name, $system ) -> $boolean +---+++ isValidWebName( $name [, $system] ) -> $boolean Check for a valid web name. If $system is true, then system web names are considered valid (names starting with _) @@ -2476,9 +2476,11 @@ sub isValidWebName { =begin TML ----++ StaticMethod isValidTopicName( $name ) -> $boolean +---++ StaticMethod isValidTopicName( $name [, $allowNonWW] ) -> $boolean Check for a valid topic name. + * =$name= - topic name + * =$allowNonWW= - true to allow non-wikiwords =cut diff --git a/core/lib/Foswiki/LoginManager.pm b/core/lib/Foswiki/LoginManager.pm index 668bc501e5..e5f6f1ba79 100644 --- a/core/lib/Foswiki/LoginManager.pm +++ b/core/lib/Foswiki/LoginManager.pm @@ -476,15 +476,9 @@ sub expireDeadSessions { opendir( D, "$Foswiki::cfg{WorkingDir}/tmp" ) || return; foreach my $file ( readdir(D) ) { - $file = Foswiki::Sandbox::untaint( - $file, - sub { - my $file = shift; - return $file if $file =~ /^(passthru|cgisess)_[0-9a-f]{32}/; - return undef; - } - ); - next unless $file; + # Validate + next unless $file =~ /^((passthru|cgisess)_[0-9a-f]{32})$/; + $file = $1; # untaint validated file name my @stat = stat("$Foswiki::cfg{WorkingDir}/tmp/$file"); @@ -522,6 +516,9 @@ sub userLoggedIn { my ( $this, $authUser, $wikiName ) = @_; my $session = $this->{session}; + if ($session->{users}) { + $session->{user} = $session->{users}->getCanonicalUserID($authUser); + } return undef if $session->inContext('command_line'); if ( $Foswiki::cfg{UseClientSessions} ) { @@ -638,6 +635,7 @@ sub _rewriteURL { # strip off existing params my $params = "?$Foswiki::LoginManager::Session::NAME=$sessionId"; + # implicit untaint is OK because recombined with url later if ( $url =~ s/\?(.*)$// ) { $params .= ';' . $1; } diff --git a/core/lib/Foswiki/LoginManager/TemplateLogin.pm b/core/lib/Foswiki/LoginManager/TemplateLogin.pm index 2b7a8bab06..726c0c6e00 100755 --- a/core/lib/Foswiki/LoginManager/TemplateLogin.pm +++ b/core/lib/Foswiki/LoginManager/TemplateLogin.pm @@ -172,7 +172,7 @@ sub login { if ($origurl =~ s/\?(.*)//) { foreach my $pair (split(/[&;]/, $1)) { if ($pair =~ /(.*?)=(.*)/) { - $query->param($1, $2); + $query->param($1, TAINT($2)); } } } diff --git a/core/lib/Foswiki/Net/HTTPResponse.pm b/core/lib/Foswiki/Net/HTTPResponse.pm index cafae85ddb..9579c843eb 100644 --- a/core/lib/Foswiki/Net/HTTPResponse.pm +++ b/core/lib/Foswiki/Net/HTTPResponse.pm @@ -19,6 +19,8 @@ See the documentation of HTTP::Response for information about the methods. package Foswiki::Net::HTTPResponse; +use Assert; + sub new { my ( $class, $message ) = @_; return bless( @@ -38,16 +40,19 @@ sub parse { $text =~ s/\r\n/\n/gs; $text =~ s/\r/\n/gs; $text =~ s/^(.*?)\n\n//s; + # untaint is OK, checked below my $httpHeader = $1; $this->{content} = $text; if ( $httpHeader =~ s/^HTTP\/[\d.]+\s(\d+)\d\d\s(.*)$// ) { $this->{code} = $1; - $this->{message} = $2; + $this->{message} = TAINT($2); } $httpHeader = "\n$httpHeader\n"; foreach my $header ( split( /\n(?=![ \t])/, $httpHeader ) ) { - if ( $header =~ /^.*?: (.*)$/s ) { - $this->{headers}->{ lc($1) } = $2; + if ( $header =~ /^(.*?): (.*)$/s ) { + # implicit untaint is OK for header names, + # but values need to be retainted + $this->{headers}->{ lc($1) } = TAINT( $2 ); } else { $this->{code} = 400; diff --git a/core/lib/Foswiki/OopsException.pm b/core/lib/Foswiki/OopsException.pm index 642bb928e0..45a9d23d6d 100644 --- a/core/lib/Foswiki/OopsException.pm +++ b/core/lib/Foswiki/OopsException.pm @@ -28,7 +28,7 @@ throw Foswiki::OopsException( 'bathplugin', status => 418, web => $web, topic => $topic, - params => [ 'big toe', 'stuck', 'hot tap' ] ); + params => [ 'big toe', 'stuck in', 'hot tap' ] ); This will raise an exception that uses the =bathplugin.tmpl= template. If =UI::run= handles the exception it will generate a redirect to: @@ -45,7 +45,7 @@ The =bathplugin.tmpl= might contain: %TMPL:DEF{"pagetitle"}%%TMPL:P{"heading"}%%TMPL:END% %TMPL:DEF{"webaction"}% *%MAKETEXT{"Warning"}%* %TMPL:END% %TMPL:DEF{"message"}% -%MAKETEXT{"Your bath cannot be filled because your [_1] is [_2] in the [_2]%TMPL:END% +%MAKETEXT{"Your bath cannot be filled because your [_1] is [_2] the [_3]%TMPL:END% In this case the =oops= page will be rendered with a 418 ("I'm a teapot") status in the HTTP header. diff --git a/core/lib/Foswiki/Plugin.pm b/core/lib/Foswiki/Plugin.pm index dcdddbb214..c17787f428 100644 --- a/core/lib/Foswiki/Plugin.pm +++ b/core/lib/Foswiki/Plugin.pm @@ -95,6 +95,7 @@ sub new { name => $name || '', module => $module, # if undef, use discovery }, $class ); +ASSERT(UNTAINTED($this->{name})); return $this; } @@ -210,7 +211,6 @@ sub registerSettings { $this->{disabled} = 1; return; } - my $prefs = $this->{session}->{prefs}; if ( !$this->{no_topic} ) { $prefs->pushPreferences( $this->{topicWeb}, $this->{name}, 'PLUGIN', diff --git a/core/lib/Foswiki/Plugins.pm b/core/lib/Foswiki/Plugins.pm index ba68ab9497..ad463e1cdd 100644 --- a/core/lib/Foswiki/Plugins.pm +++ b/core/lib/Foswiki/Plugins.pm @@ -165,8 +165,17 @@ sub load { unless ($allDisabled) { if ( $query && defined( $query->param('debugenableplugins') ) ) { - @pluginList = - split( /[,\s]+/, $query->param('debugenableplugins') ); + foreach my $pn (split( + /[,\s]+/, $query->param('debugenableplugins') )) { + push (@pluginList, Foswiki::Sandbox::untaint( + $pn, + sub { + throw Error::Simple( + 'Bad debugenableplugins') unless + $pn =~ /^\w+$/; + return $pn; + })); + } } else { if ( $Foswiki::cfg{PluginsOrder} ) { diff --git a/core/lib/Foswiki/Render.pm b/core/lib/Foswiki/Render.pm index b58947dc4b..13fd4835c7 100644 --- a/core/lib/Foswiki/Render.pm +++ b/core/lib/Foswiki/Render.pm @@ -356,6 +356,8 @@ sub _emitTR { push( @attr, align => 'center' ); } } + # implicit untaint is OK, because we are just taking topic data + # and rendering it; no security step is bypassed. if (/^\s*\*(.*)\*\s*$/) { $cells .= CGI::th( {@attr}, CGI::strong(" $1 ") ) . "\n"; } @@ -810,6 +812,7 @@ s/(?<=[\s\(])($Foswiki::regex{wikiWordRegex}|[$Foswiki::regex{upperAlpha}])/