Merge branch 'Release02x01'

foswiki · May 16, 2016 · 8211771 · 8211771
2 parents 4f9ecbb + 2679175
commit 8211771
Show file tree

Hide file tree

Showing 3 changed files with 121 additions and 2 deletions.
diff --git a/UnitTestContrib/lib/Foswiki/Contrib/UnitTestContrib/MANIFEST b/UnitTestContrib/lib/Foswiki/Contrib/UnitTestContrib/MANIFEST
@@ -96,6 +96,7 @@ test/unit/RequestTests.pm 0644
 test/unit/ResponseTests.pm 0644
 test/unit/RobustnessTests.pm 0644
 test/unit/SaveScriptTests.pm 0644
+test/unit/SecurityTests.pm 0644
 test/unit/SeleniumConfigTests.pm 0644
 test/unit/SemiAutomaticTestCaseTests.pm 0644
 test/unit/SerialiseTests.pm 0644

diff --git a/UnitTestContrib/test/unit/SecurityTests.pm b/UnitTestContrib/test/unit/SecurityTests.pm
@@ -0,0 +1,117 @@
+package SecurityTests;
+use FoswikiFnTestCase();
+use Foswiki::UI::Attach();
+our @ISA = qw( FoswikiFnTestCase );
+
+# use strict;
+
+my $session;    # Foswiki instance
+
+sub new {
+    my $self = shift()->SUPER::new(@_);
+
+    return $self;
+}
+
+sub set_up {
+    my $this = shift;
+    $this->SUPER::set_up();
+    $session = undef;
+}
+
+sub create_session {
+    my $this       = shift;
+    my $query_opts = shift;
+
+    # a simple query using attach
+    my $query = new Unit::Request($query_opts);
+    $query->path_info("/$this->{test_web}/$this->{test_topic}");
+    $query->action("attach");
+
+    # Create a Foswiki instance
+    $session =
+      $this->createNewFoswikiSession( $this->{test_user_login}, $query );
+
+    return $session;
+}
+
+sub tear_down {
+    my $this = shift;    # the Test::Unit::TestCase object
+
+    if ($session) {
+
+        # FoswikiFnTestCase does most of this
+        1;
+    }
+
+    # This will automatically restore the state of $Foswiki::cfg
+    $this->SUPER::tear_down();
+}
+
+sub test_setup {
+
+    # if this test fails, there may be something wrong with the design of
+    # other tests testing real issues.
+
+    my $this = shift;
+
+    $this->create_session( { filename => ["goober"] } );
+    my $query = $this->{request};
+
+    $this->assert_str_equals( "attach",          $query->action() );
+    $this->assert_str_equals( "filename=goober", $query->queryString() );
+    $this->assert_str_equals( "goober", scalar( $query->param('filename') ) );
+
+    # print $query->url(-query => 1), "\n";
+
+    my ( $respText, $result, $stdout, $stderr ) = $this->captureWithKey(
+        attach => sub {
+            no strict 'refs';
+            Foswiki::UI::Attach::attach( $this->{session} );
+            use strict 'refs';
+            $Foswiki::engine->finalize( $this->{session}{response},
+                $this->{session}{request} );
+        }
+    );
+
+    # print $respText, "\n";
+
+    $this->assert_matches( qr/<input [^>]* value="goober"/, $respText );
+
+}
+
+sub test_attach_filename_xss {
+
+    my $this = shift;
+
+    # send filename="><sCrIpT>alert(66562)</sCrIpT>
+    $this->create_session(
+        { filename => ['"><sCrIpT>alert(66562)</sCrIpT>'] } );
+    my $query = $this->{request};
+
+    # print $query->url(-query => 1), "\n";
+
+    my ( $respText, $result, $stdout, $stderr ) = $this->captureWithKey(
+        attach => sub {
+            no strict 'refs';
+            Foswiki::UI::Attach::attach( $this->{session} );
+            use strict 'refs';
+            $Foswiki::engine->finalize( $this->{session}{response},
+                $this->{session}{request} );
+        }
+    );
+
+    # print $respText, "\n";
+
+    # our filename got it in in some form...
+    $this->assert_matches( qr/sCrIpT/, $respText,
+        "Expected to see harmless trace of filename (sCrIpT)" );
+
+    # ...but must not allow pop-up alert
+    $this->assert_does_not_match( qr/<sCrIpT>alert\(66562\)<\/sCrIpT>/,
+        $respText,
+        "Detected Javascript injection: <sCrIpT>alert\(66562\)<\/sCrIpT>" );
+
+}
+
+1;
diff --git a/core/lib/Foswiki/UI/Attach.pm b/core/lib/Foswiki/UI/Attach.pm
@@ -96,12 +96,13 @@ sub attach {
     }
     $tmpl =~ s/%ATTACHTABLE%/$atext/g;
     $tmpl =~ s/%FILEUSER%/$fileWikiUser/g;
-    $tmpl =~ s/%FILENAME%/$fileName/g;
+    $args->{name} = Foswiki::entityEncode( $args->{name} );
+    $tmpl =~ s/%FILENAME%/$args->{name}/g;
     $tmpl = $topicObject->expandMacros($tmpl);
     $tmpl = $topicObject->renderTML($tmpl);
     $tmpl =~ s/%HIDEFILE%/$isHideChecked/g;
 
-    my $filePath = $args->{path} || $fileName;
+    my $filePath = Foswiki::entityEncode( $args->{path} ) || $args->{name};
     $tmpl =~ s/%FILEPATH%/$filePath/g;
     $args->{comment} = Foswiki::entityEncode( $args->{comment} );
     $tmpl =~ s/%FILECOMMENT%/$args->{comment}/g;