From 73a51bb4fbd5ceb4f466bb41ede1304f363cb6b6 Mon Sep 17 00:00:00 2001
From: Xavier Guimard <xavier.guimard@laposte.net>
Date: Mon, 23 May 2016 11:53:09 +0000
Subject: [PATCH] Working on XSS detection (#595)

---
 .../lib/Lemonldap/NG/Portal/Main/Init.pm      |  36 ++--
 .../lib/Lemonldap/NG/Portal/Main/Run.pm       |   2 +-
 lemonldap-ng-portal/t/03-XSS-protection.t     | 177 +++++++++---------
 3 files changed, 101 insertions(+), 114 deletions(-)

diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Init.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Init.pm
index 0c4672485..7efe14cbf 100644
--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Init.pm
+++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Init.pm
@@ -30,7 +30,7 @@ has _macros => ( is => 'rw' );
 has _groups => ( is => 'rw' );
 
 # TrustedDomain regexp
-has trustedDomains => ( is => 'rw' );
+has trustedDomainsRe => ( is => 'rw' );
 
 # Lists to store plugins entry-points
 has beforeAuth => (
@@ -120,7 +120,7 @@ sub reloadConf {
       $self->conf->{templateDir} . '/' . $self->conf->{portalSkin};
 
     $self->{staticPrefix} = $self->conf->{staticPrefix} || '/';
-    $self->{languages} = $self->conf->{languages} || '/';
+    $self->{languages}    = $self->conf->{languages}    || '/';
 
     # Initialize session DBs
     unless ( $self->conf->{globalStorage} ) {
@@ -166,17 +166,25 @@ sub reloadConf {
     if (    $self->conf->{trustedDomains}
         and $self->conf->{trustedDomains} =~ /^\s*\*\s*$/ )
     {
-        $self->trustedDomains(qr#^https?://#);
+        $self->trustedDomainsRe(qr#^https?://#);
     }
     else {
         my $re = Regexp::Assemble->new();
         if ( my $td = $self->conf->{trustedDomains} ) {
             $td =~ s/^\s*(.*?)\s*/$1/;
-            $self->lmLog( "Domain $_ added in trusted domains", 'debug' );
             foreach ( split( /\s+/, $td ) ) {
+                next unless($td);
                 s#^\.#([^/]+\.)?#;
-                s/\./\\./;
-                $re->add($_);
+                $self->lmLog( "Domain $_ added in trusted domains", 'debug' );
+                s/\./\\./g;
+
+                # This regexp is valid for the followings hosts:
+                #  - $td
+                #  - $domainlabel.$td
+                # $domainlabel is build looking RFC2396
+                # (see Regexp::Common::URI::RFC2396)
+                $_ =~ s/\*\\\./(?:(?:[a-zA-Z0-9][-a-zA-Z0-9]*)?[a-zA-Z0-9]\\.)*/g;
+                $re->add("$_");
             }
         }
         foreach my $vhost ( keys %{ $self->conf->{locationRules} } ) {
@@ -192,20 +200,8 @@ sub reloadConf {
                 }
             }
         }
-        my $tmp = 'https?://' . $re->as_string . '(?:/|$)';
-        $self->trustedDomains(qr/$tmp/);
-    }
-    if ( my $td = $self->conf->{trustedDomains} ) {
-        $td =~ s/^\s*(.*?)\s*/$1/;
-        if ( $td eq '*' ) {
-            $self->trustedDomains(qr#^https?://#);
-        }
-        else {
-            my $tmp =
-              join( '|', map { s#^\.#([^/]+\.)?# } split( /\s+/, $td ) );
-            $tmp =~ s/\./\\./g;
-            $self->trustedDomains(qr#^https?://$tmp(?:\d+)?(?:/|$)#);
-        }
+        my $tmp = 'https?://' . $re->as_string . '(?::\d+)?(?:/|$)';
+        $self->trustedDomainsRe(qr/$tmp/);
     }
 
     # Compile macros in _macros, groups in _groups
diff --git a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm
index b75eb8540..f1285fd75 100644
--- a/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm
+++ b/lemonldap-ng-portal/lib/Lemonldap/NG/Portal/Main/Run.pm
@@ -332,7 +332,7 @@ sub _md5hash {
 # trusted domain
 sub isTrustedUrl {
     my ( $self, $url ) = @_;
-    return $url =~ $self->trustedDomains ? 1 : 0;
+    return $url =~ $self->trustedDomainsRe ? 1 : 0;
 }
 
 1;
diff --git a/lemonldap-ng-portal/t/03-XSS-protection.t b/lemonldap-ng-portal/t/03-XSS-protection.t
index 2d5bf9230..5eb277555 100644
--- a/lemonldap-ng-portal/t/03-XSS-protection.t
+++ b/lemonldap-ng-portal/t/03-XSS-protection.t
@@ -1,150 +1,141 @@
-# Before `make install' is performed this script should be runnable with
-# `make test'. After `make install' it should work as `perl Lemonldap-NG-Portal.t'
-
-#########################
-
-# change 'tests => 1' to 'tests => last_test_to_print';
-
-package My::Portal;
+use Test::More;
 use strict;
-use Test::More tests => 22;
+use IO::String;
 
 BEGIN {
-    use_ok( 'Lemonldap::NG::Portal::Simple', ':all' );
-    sub Lemonldap::NG::Portal::Simple::lmLog { }
+    use_ok( 'Lemonldap::NG::Portal::Main::Constants', ':all' );
 }
 
-#use Lemonldap::NG::Portal::Simple;
+require 't/test-lib.pm';
 
-our @ISA = 'Lemonldap::NG::Portal::Simple';
-my ( $url, $result, $logout );
-$logout = 0;
-my @h = (
+init( { useSafeJail => 1, trustedDomains => 'example3.com *.example2.com' } );
 
-    '' => PE_OK, 'Empty',
+my @tests = (
 
-    # 4 http://test.example.com/
-    'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb20v' => PE_OK, 'Protected virtual host',
+    # 1 No redirection
+    '' => 0, 'Empty',
 
-    # 5 http://test.example.com
-    'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb20v' => PE_OK, 'Missing / in URL',
+    # 2 http://test1.example.com/
+    'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29tLw==' => 1, 'Protected virtual host',
 
-    # 6 http://test.example.com:8000/test
-    'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb206ODAwMC90ZXN0' => PE_OK, 'Non default port',
+    # 3 http://test1.example.com
+    'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29t' => 1, 'Missing / in URL',
 
-    # 7 http://test.example.com:8000/
-    'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb206ODAwMA==' => PE_BADURL,
+    # 4 http://test1.example.com:8000/test
+    'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29tOjgwMDAvdGVzdA==' => 1,
+    'Non default port',
+
+    # 5 http://test1.example.com:8000/
+    'aHR0cDovL3Rlc3QxLmV4YW1wbGUuY29tOjgwMDAv' => 1,
     'Non default port with missing /',
 
-    # 8 http://t.example2.com/test
-    'aHR0cDovL3QuZXhhbXBsZTIuY29tL3Rlc3Q=' => PE_OK,
+    # 6 http://t.example2.com/test
+    'aHR0cDovL3QuZXhhbXBsZTIuY29tL3Rlc3Q=' => 1,
     'Undeclared virtual host in trusted domain',
 
-    # 9 http://testexample2.com/
-    'aHR0cDovL3Rlc3RleGFtcGxlMi5jb20vCg==' => PE_BADURL,
+    # 7 http://testexample2.com/
+    'aHR0cDovL3Rlc3RleGFtcGxlMi5jb20vCg==' => 0,
     'Undeclared virtual host in untrusted domain'
       . ' (looks like a trusted domain, but is not)',
 
-    # 10 http://test.example3.com/
-    'aHR0cDovL3Rlc3QuZXhhbXBsZTMuY29tLwo=' => PE_BADURL,
+    # 8 http://test.example3.com/
+    'aHR0cDovL3Rlc3QuZXhhbXBsZTMuY29tLwo=' => 0,
     'Undeclared virtual host in untrusted domain (domain name'
       . ' "example3.com" is trusted, but domain "*.example3.com" not)',
 
-    # 11 http://example3.com/
-    'aHR0cDovL2V4YW1wbGUzLmNvbS8K' => PE_OK,
+    # 9 http://example3.com/
+    'aHR0cDovL2V4YW1wbGUzLmNvbS8K' => 1,
     'Undeclared virtual host with trusted domain name',
 
-    # 12 http://t.example.com/test
-    'aHR0cDovL3QuZXhhbXBsZS5jb20vdGVzdA==' => PE_BADURL,
+    # 10 http://t.example.com/test
+    'aHR0cDovL3QuZXhhbXBsZS5jb20vdGVzdA==' => 0,
     'Undeclared virtual host in (untrusted) protected domain',
 
-    # 13
-    'http://test.com/' => PE_BADURL, 'Non base64 encoded characters',
+    # 11
+    'http://test.com/' => 0, 'Non base64 encoded characters',
 
-    # 14 http://test.example.com:8000V
-    'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb206ODAwMFY=' => PE_BADURL,
+    # 12 http://test.example.com:8000V
+    'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb206ODAwMFY=' => 0,
     'Non number in port',
 
-    # 15 http://t.ex.com/test
-    'aHR0cDovL3QuZXguY29tL3Rlc3Q=' => PE_BADURL,
+    # 13 http://t.ex.com/test
+    'aHR0cDovL3QuZXguY29tL3Rlc3Q=' => 0,
     'Undeclared virtual host in untrusted domain',
 
-    # 16 http://test.example.com/%00
-    'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb20vJTAw' => PE_BADURL, 'Base64 encoded \0',
+    # 14 http://test.example.com/%00
+    'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb20vJTAw' => 0, 'Base64 encoded \0',
 
-    # 17 http://test.example.com/test\0
-    'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb20vdGVzdAA=' => PE_BADURL,
+    # 15 http://test.example.com/test\0
+    'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb20vdGVzdAA=' => 0,
     'Base64 and url encoded \0',
 
-    # 18
-    'XX%00' => PE_BADURL, 'Non base64 encoded \0 ',
+    # 16
+    'XX%00' => 0, 'Non base64 encoded \0 ',
 
-    # 19 http://test.example.com/test?<script>alert()</script>
+    # 17 http://test.example.com/test?<script>alert()</script>
     'aHR0cDovL3Rlc3QuZXhhbXBsZS5jb20vdGVzdD88c2NyaXB0PmFsZXJ0KCk8L3NjcmlwdD4='
-      => PE_BADURL,
+      => 0,
     'base64 encoded HTML tags',
 
     # LOGOUT TESTS
     'LOGOUT',
 
-    # 20 url=http://www.toto.com/, bad referer
+    # 18 url=http://www.toto.com/, bad referer
     'aHR0cDovL3d3dy50b3RvLmNvbS8=',
-    'http://bad.com/' => PE_BADURL,
+    'http://bad.com/' => 0,
     'Logout required by bad site',
 
-    # 21 url=http://www.toto.com/, good referer
+    # 19 url=http://www.toto.com/, good referer
     'aHR0cDovL3d3dy50b3RvLmNvbS8=',
-    'http://test.example.com/' => PE_OK,
+    'http://test.example.com/' => 1,
     'Logout required by good site',
 
-    # 22 url=http://www?<script>, good referer
+    # 20 url=http://www?<script>, good referer
     'aHR0cDovL3d3dz88c2NyaXB0Pg==',
-    'http://test.example.com/' => PE_BADURL,
+    'http://test.example.com/' => 0,
     'script with logout',
 );
 
-my $count = 0;
+my $res;
+ok(
+    $res = &client->_post(
+        '/', '',
+        IO::String->new('user=dwho&password=dwho'),
+        'application/x-www-form-urlencoded', 23
+    ),
+    'Auth query'
+);
+ok( $res->[0] == 200, 'Response is 200' ) or explain( $res->[0], 200 );
+my $id;
+ok( $id = getCookies($res)->{lemonldap}, 'Get LLNG cookie' )
+  or explain( $res, 'Set-Cookie: something' );
 
-sub param {
-    shift;
-    my $p = shift;
-    $count++;
-    if ( $p and $p eq 'url' ) {
-        return $url;
-    }
-    else {
-        return $logout;
-    }
+count(4);
+
+while ( defined( my $url = shift(@tests) ) ) {
+    last if ( $url eq 'LOGOUT' );
+    my $redir  = shift @tests;
+    my $detail = shift @tests;
+    ok(
+        $res = &client->_get(
+            '/',
+            query  => "url=$url",
+            cookie => "lemonldap=$id",
+
+            accept => 'text/html'
+        ),
+        $detail
+    );
+    ok( ( $res->[0] == ( $redir ? 302 : 200 ) ),
+        ( $redir ? 'Get redirection' : 'Redirection dropped' ) )
+      or explain( $res->[0], ( $redir ? 302 : 200 ) );
+    count(2);
 }
 
-my $p;
-
-# CGI Environment
-$ENV{SCRIPT_NAME}     = '/test.pl';
-$ENV{SCRIPT_FILENAME} = '/tmp/test.pl';
-$ENV{REQUEST_METHOD}  = 'GET';
-$ENV{REQUEST_URI}     = "/test.pl";
-$ENV{QUERY_STRING}    = "";
-
-ok(
-    $p = My::Portal->new(
-        {
-            globalStorage  => 'Apache::Session::File',
-            domain         => 'example.com',
-            authentication => 'LDAP test=1',
-            userDB         => 'Null',
-            passwordDB     => 'Null',
-            registerDB     => 'Null',
-            domain         => 'example.com',
-            trustedDomains => '.example2.com example3.com',
-            checkXSS       => 1,
-        }
-    ),
-    'Portal object'
-);
-
-$p->{reVHosts} = '(?:test\.example\.com)';
+clean_sessions();
 
+done_testing( count() );
+__END__
 while ( defined( $url = shift(@h) ) ) {
     last if ( $url eq 'LOGOUT' );
     $result = shift @h;