[Rt-commit] rt branch, 4.2/csrf-whitelist, created. rt-4.2.11-38-gd2c0f0d

[Dustin Graves]

The branch, 4.2/csrf-whitelist has been created
        at  d2c0f0d81b2c3ab2a47c0c66b0c8642b1bf2c5ca (commit)

- Log -----------------------------------------------------------------
commit d2c0f0d81b2c3ab2a47c0c66b0c8642b1bf2c5ca
Author: Dustin Graves <dustin at bestpractical.com>
Date:   Mon Aug 3 16:40:24 2015 -0400

    add CSRF whitelist for component parameters
    
    in particular, /Search/Build.html param SavedSearchLoad is whitelisted,
    but not other parameters
    
    move parameter whitelist logic into own sub
    
    Fixes: I#31090

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 413c165..c5cb43f 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -1376,6 +1376,29 @@ our %is_whitelisted_component = (
     '/Ticket/ShowEmailRecord.html' => 1,
 );
 
+# Whitelist arguments that do not indicate an effectful request.
+our @whitelisted_args = (
+    # For example, "id" is acceptable because that is how RT retrieves a
+    # record.
+    'id',
+
+    # If they have a results= from MaybeRedirectForResults, that's also fine.
+    'results',
+
+    # The homepage refresh, which uses the Refresh header, doesn't send
+    # a referer in most browsers; whitelist the one parameter it reloads
+    # with, HomeRefreshInterval, which is safe
+    'HomeRefreshInterval',
+
+    # The NotMobile flag is fine for any page; it's only used to toggle a flag
+    # in the session related to which interface you get.
+    'NotMobile',
+);
+
+our %whitelisted_component_parameters = (
+    '/Search/Build.html' => ['SavedSearchLoad'],
+);
+
 # Components which are blacklisted from automatic, argument-based whitelisting.
 # These pages are not idempotent when called with just an id.
 our %is_blacklisted_component = (
@@ -1420,28 +1443,23 @@ sub IsCompCSRFWhitelisted {
         }
     }
 
-    # Eliminate arguments that do not indicate an effectful request.
-    # For example, "id" is acceptable because that is how RT retrieves a
-    # record.
-    delete $args{id};
+    return AreCompCSRFParametersWhitelisted($comp, \%args);
+}
 
-    # If they have a results= from MaybeRedirectForResults, that's also fine.
-    delete $args{results};
+sub AreCompCSRFParametersWhitelisted {
+    my $sub = shift;
+    my $ARGS = shift;
 
-    # The homepage refresh, which uses the Refresh header, doesn't send
-    # a referer in most browsers; whitelist the one parameter it reloads
-    # with, HomeRefreshInterval, which is safe
-    delete $args{HomeRefreshInterval};
+    my %leftover_args = %{ $ARGS };
 
-    # The NotMobile flag is fine for any page; it's only used to toggle a flag
-    # in the session related to which interface you get.
-    delete $args{NotMobile};
+    # Join global whitelist and component-specific whitelist
+    my @comp_whitelisted_args = (@whitelisted_args, @{$whitelisted_component_parameters{$sub}});
 
-    # If there are no arguments, then it's likely to be an idempotent
-    # request, which are not susceptible to CSRF
-    return 1 if !%args;
+    for my $arg (@comp_whitelisted_args) {
+        delete $leftover_args{$arg};
+    }
 
-    return 0;
+    return !%leftover_args;
 }
 
 sub IsRefererCSRFWhitelisted {

-----------------------------------------------------------------------