changelog | [dw-free] http://bugs.dwscoalition.org/show

[commit: http://hg.dwscoalition.org/dw-free/rev/116559a9f5bb]

http://bugs.dwscoalition.org/show_bug.cgi?id=2735

Stricter handling of suffixes in page referers.

Patch by
fu.

Files modified:
cgi-bin/weblib.pl
htdocs/admin/vgifts/inactive.bml
htdocs/admin/vgifts/index.bml
htdocs/admin/vgifts/tags.bml
t/referer.t
--------------------------------------------------------------------------------
diff -r 2127ee88cfd3 -r 116559a9f5bb cgi-bin/weblib.pl
--- a/cgi-bin/weblib.pl	Tue May 17 10:15:43 2011 +0800
+++ b/cgi-bin/weblib.pl	Tue May 17 10:20:10 2011 +0800
@@ -757,11 +757,19 @@
     # escape any regex characters, like the '.' in '.bml'
     $uri = quotemeta( $uri );
 
+    # check that the end of the uri matches exactly (no extra characters or dir levels)
+    # or else that the uri is followed immediately by additional parameters
+    my $checkend = '(?:$|\\?)';
+
     # allow us to properly check URIs without .bml extensions
     if ( $origuri =~ /\.bml($|\?)/ ) {
-      my $checkend = ( $1 eq '?' ? '' : '(?:$|\\?)' );
-      $uri     =~ s/\\.bml($|\\\?)/$1$checkend/;
-      $referer =~ s/\.bml($|\?)/$1/;
+        $checkend = '' if $1 eq '?';
+        $uri     =~ s/\\.bml($|\\\?)/$1$checkend/;
+        $referer =~ s/\.bml($|\?)/$1/;
+    } elsif ( $uri ) {
+        $uri .= $checkend;
+    } else {
+        $uri = '(/|$)';
     }
 
     return 1 if $LJ::SITEROOT   && $referer =~ m!^\Q$LJ::SITEROOT\E$uri!;
diff -r 2127ee88cfd3 -r 116559a9f5bb htdocs/admin/vgifts/inactive.bml
--- a/htdocs/admin/vgifts/inactive.bml	Tue May 17 10:15:43 2011 +0800
+++ b/htdocs/admin/vgifts/inactive.bml	Tue May 17 10:20:10 2011 +0800
@@ -1,13 +1,14 @@
 <?_code
 {
     use strict;
-    use vars qw($title $body $page $action %GET %POST);
+    use vars qw($title $body $page $baseuri $action %GET %POST);
 
     use DW::VirtualGift;
 
     $title = $ML{'.title'};
     $body = "";
-    $page = "$LJ::SITEROOT/admin/vgifts/inactive";
+    $baseuri = "/admin/vgifts/inactive";
+    $page = "$LJ::SITEROOT$baseuri";
     $action = 'inactive';  # for $postform
 
     # helper routines
@@ -21,9 +22,7 @@
 
     my $loose_refer = sub {
         return 1 unless my $refer = BML::get_client_header('Referer');
-        my ( $getargs ) = ( $refer =~ m/\?(.*)$/ );
-        return LJ::check_referer( $page ) unless $getargs;
-        return LJ::check_referer( "$page?$getargs" );
+        return LJ::check_referer( $baseuri );
     };
 
     my $strict_refer = sub {
diff -r 2127ee88cfd3 -r 116559a9f5bb htdocs/admin/vgifts/index.bml
--- a/htdocs/admin/vgifts/index.bml	Tue May 17 10:15:43 2011 +0800
+++ b/htdocs/admin/vgifts/index.bml	Tue May 17 10:20:10 2011 +0800
@@ -1,13 +1,14 @@
 <?_code
 {
     use strict;
-    use vars qw($title $body $page $action %GET %POST);
+    use vars qw($title $body $page $baseuri $action %GET %POST);
 
     use DW::VirtualGift;
 
     $title = $ML{'.title'};
     $body = "";
-    $page = "$LJ::SITEROOT/admin/vgifts/";
+    $baseuri = "/admin/vgifts/";
+    $page = "$LJ::SITEROOT$baseuri";
     $action = 'index';  # for $postform
 
     # helper routines
@@ -21,18 +22,15 @@
 
     my $loose_refer = sub {
         return 1 unless my $refer = BML::get_client_header('Referer');
-        my ( $getargs ) = ( $refer =~ m/\?(.*)$/ );
         # annoyingly, we get different results for /index vs /
-        return LJ::check_referer( $page ) ||
-            LJ::check_referer( "${page}index" ) unless $getargs;
-
-        return LJ::check_referer( "$page?$getargs" ) ||
-            LJ::check_referer( "${page}index?$getargs" );
+        return LJ::check_referer( $baseuri ) ||
+            LJ::check_referer( "${$baseuri}index" );
     };
 
     my $strict_refer = sub {
         # make sure we have a referer header. check_referer doesn't care.
-        return BML::get_client_header('Referer') && $loose_refer->();
+        my $ret = BML::get_client_header('Referer') && $loose_refer->();
+        return $ret;
     };
 
     my $imgposted = sub {
diff -r 2127ee88cfd3 -r 116559a9f5bb htdocs/admin/vgifts/tags.bml
--- a/htdocs/admin/vgifts/tags.bml	Tue May 17 10:15:43 2011 +0800
+++ b/htdocs/admin/vgifts/tags.bml	Tue May 17 10:20:10 2011 +0800
@@ -1,13 +1,14 @@
 <?_code
 {
     use strict;
-    use vars qw($title $body $page $action %GET %POST);
+    use vars qw($title $body $page $baseuri $action %GET %POST);
 
     use DW::VirtualGift;
 
     $title = $ML{'.title'};
     $body = "";
-    $page = "$LJ::SITEROOT/admin/vgifts/tags";
+    $baseuri = "/admin/vgifts/tags";
+    $page = "$LJ::SITEROOT$baseuri";
     $action = 'tags';  # for $postform
 
     # helper routines
@@ -21,9 +22,7 @@
 
     my $loose_refer = sub {
         return 1 unless my $refer = BML::get_client_header('Referer');
-        my ( $getargs ) = ( $refer =~ m/\?(.*)$/ );
-        return LJ::check_referer( $page ) unless $getargs;
-        return LJ::check_referer( "$page?$getargs" );
+        return LJ::check_referer( $baseuri );
     };
 
     my $strict_refer = sub {
diff -r 2127ee88cfd3 -r 116559a9f5bb t/referer.t
--- a/t/referer.t	Tue May 17 10:15:43 2011 +0800
+++ b/t/referer.t	Tue May 17 10:20:10 2011 +0800
@@ -6,7 +6,7 @@
 require 'ljlib.pl';
 require 'weblib.pl';
 
-plan tests => 18;
+plan tests => 25;
 
 {
     note( '$LJ::SITEROOT not set up. Setting up for the test.' ) unless $LJ::SITEROOT;
@@ -14,39 +14,57 @@
 
     # first argument is the page we want to check against (system-provided)
     # second argument is the page the user said they were coming from
+
+    note( "basic tests" );
     ok( LJ::check_referer( "/page.bml", "$LJ::SITEROOT/page.bml" ), "Visited page with bml extension; uri check has .bml." );
     ok( LJ::check_referer( "/page.bml", "$LJ::SITEROOT/page" ), "Visited page with no bml extension; uri check has .bml" );
-    ok( LJ::check_referer( "/page", "$LJ::SITEROOT/page.bml" ), "Visited page with bml extension; uri check has no .bml" );
     ok( LJ::check_referer( "/page", "$LJ::SITEROOT/page" ), "Visited page with no bml extension; uri check has .bml" );
 
 
+    note( "checking ssl" );
     note( '$LJ::SSLROOT not set up. Setting up for the test.' ) unless $LJ::SSLROOT;
     $LJ::SSLROOT ||= "https://$LJ::DOMAIN_WEB";
 
     ok( LJ::check_referer( "/page", "$LJ::SSLROOT/page" ), "Checking the SSLROOT" );
 
 
+    note( "checking domain / siteroot " );
     my $somerandomsiteroot = "http://www.somerandomsite.org";
     ok( LJ::check_referer( "", $LJ::SITEROOT ), "Check if SITEROOT is on our site" );
     ok( LJ::check_referer( "", "$LJ::SITEROOT/page" ), "Check if any page on our site is on our site" );
     ok( LJ::check_referer( "", $LJ::SSLROOT ), "Check if SSLROOT is on our site" );
     ok( ! LJ::check_referer( "", $somerandomsiteroot ), "Check if somerandomsite is on our site" );
+    ok( ! LJ::check_referer( "", "${LJ::SITEROOT}.other.tld" ), "Check if another site which begins with our SITEROOT is on our site" );
+    ok( ! LJ::check_referer( "/page", "/page" ), "Passed in a bare URI as a referer" );
 
+
+    note( "checking extensions" );
     ok( ! LJ::check_referer( "/page.bml", "$LJ::SITEROOT/page.bmls" ), "Visited page with invalid extension .bmls; uri should be page.bml." );
     ok( ! LJ::check_referer( "/page.bml", "$LJ::SITEROOT/page.html" ), "Visited page with invalid extension .html; uri should be page.bml." );
 
+    ok( ! LJ::check_referer( "/page", "$LJ::SITEROOT/page.bml" ), "Visited page with bml extension; uri check has no .bml" );
+    ok( ! LJ::check_referer( "/page", "$LJ::SITEROOT/page.bmls" ), "Visited page with invalid extension .bmls (bml+suffix)" );
+    ok( ! LJ::check_referer( "/page", "$LJ::SITEROOT/page.html" ), "Visited page with invalid extension .html (nothing that looks like bml)" );
 
-    ok( LJ::check_referer( "/page", "$LJ::SITEROOT/page.bmls" ), "Visited page with invalid extension .bmls; uri can be page.*" );
-    ok( LJ::check_referer( "/page", "$LJ::SITEROOT/page.html" ), "Visited page with invalid extension .html; uri can be page.*" );
 
-
-    ok( ! LJ::check_referer( "/page", "/page" ), "Passed in a bare URI as a referer" );
+    note( "checking for partial matches (should not match)" );
     ok( ! LJ::check_referer( "/page", "$LJ::SITEROOT/prefix-page" ), "Visited URL does not match referer URL. (Added prefix)" );
-    ok( LJ::check_referer( "/page", "$LJ::SITEROOT/page?argument" ), "Visited URL matches referer URL (with arguments)" );
-
+    ok( ! LJ::check_referer( "/page", "$LJ::SITEROOT/page-suffix" ), "Visited URL does not match referer URL. (Added suffix)" );
+    ok( ! LJ::check_referer( "/page", "$LJ::SITEROOT/page/other" ), "Visited URL does not match referer URL. (Added directory level)" );
 
     ok( ! LJ::check_referer( "/page", "$LJ::SITEROOT/" ), "Visited bare SITEROOT" );
     ok( ! LJ::check_referer( "/page", "$somerandomsiteroot/page" ), "Visited SITEROOT is not from our domain" );
+
+
+    note( "checking for URL arguments" );
+    # Argument tests where uri does not have an argument
+    ok( LJ::check_referer( "/page", "$LJ::SITEROOT/page?argument" ), "Visited URL matches referer URL (with arguments)" );
+    ok( ! LJ::check_referer( "/page", "$LJ::SITEROOT/page.bml?argument" ), "Visited .bml URL with arguments matches allowed URL" );
+    ok( LJ::check_referer( "/page.bml", "$LJ::SITEROOT/page?argument" ), "Visited non-bml URL with arguments matches allowed .bml URL" );
+    ok( LJ::check_referer( "/page.bml", "$LJ::SITEROOT/page.bml?argument" ), "Visited .bml URL with arguments matches allowed .bml URL" );
+
+    # Tricks with two question marks in referer
+    ok( LJ::check_referer( "/page", "$LJ::SITEROOT/page?argument?suffix" ), "Visited page has second question mark followed by suffix; uri check has no arguments" );
 }
 
 1;
--------------------------------------------------------------------------------
[dw-free] http://bugs.dwscoalition.org/show_bug.cgi?id=2735
