rt 4.2.13 ticket#13852
[freeside.git] / rt / lib / RT / Interface / Web.pm
index dad6a8e..f26afde 100644 (file)
@@ -2,7 +2,7 @@
 #
 # COPYRIGHT:
 #
-# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
+# This software is Copyright (c) 1996-2016 Best Practical Solutions, LLC
 #                                          <sales@bestpractical.com>
 #
 # (Except where explicitly superseded by other copyright notices)
@@ -1313,13 +1313,13 @@ sub ValidateWebConfig {
     if ( $port != RT->Config->Get('WebPort') and not $ENV{'rt.explicit_port'}) {
         $RT::Logger->warn("The requested port ($port) does NOT match the configured WebPort ($RT::WebPort).  "
                          ."Perhaps you should Set(\$WebPort, $port); in RT_SiteConfig.pm, "
-                         ."otherwise your internal links may be broken.");
+                         ."otherwise your internal hyperlinks may be broken.");
     }
 
     if ( $host ne RT->Config->Get('WebDomain') ) {
         $RT::Logger->warn("The requested host ($host) does NOT match the configured WebDomain ($RT::WebDomain).  "
                          ."Perhaps you should Set(\$WebDomain, '$host'); in RT_SiteConfig.pm, "
-                         ."otherwise your internal links may be broken.");
+                         ."otherwise your internal hyperlinks may be broken.");
     }
 
     return; #next warning flooding our logs, doesn't seem applicable to our use
@@ -1333,7 +1333,7 @@ sub ValidateWebConfig {
     if ($ENV{SCRIPT_NAME} ne RT->Config->Get('WebPath') and not $proxied) {
         $RT::Logger->warn("The requested path ($ENV{SCRIPT_NAME}) does NOT match the configured WebPath ($RT::WebPath).  "
                          ."Perhaps you should Set(\$WebPath, '$ENV{SCRIPT_NAME}'); in RT_SiteConfig.pm, "
-                         ."otherwise your internal links may be broken.");
+                         ."otherwise your internal hyperlinks may be broken.");
     }
 }
 
@@ -1364,7 +1364,7 @@ sub StaticRoots {
     return grep { $_ and -d $_ } @static;
 }
 
-our %is_whitelisted_component = (
+our %IS_WHITELISTED_COMPONENT = (
     # The RSS feed embeds an auth token in the path, but query
     # information for the search.  Because it's a straight-up read, in
     # addition to embedding its own auth, it's fine.
@@ -1386,9 +1386,40 @@ our %is_whitelisted_component = (
     '/Ticket/ShowEmailRecord.html' => 1,
 );
 
+# Whitelist arguments that do not indicate an effectful request.
+our @GLOBAL_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_ARGS = (
+    # SavedSearchLoad - This happens when you middle-(or ⌘ )-click "Edit" for a saved search on
+    # the homepage. It's not going to do any damage
+    # NewQuery - This is simply to clear the search query
+    '/Search/Build.html' => ['SavedSearchLoad','NewQuery'],
+    # Happens if you try and reply to a message in the ticket history or click a number
+    # of options on a tickets Action menu
+    '/Ticket/Update.html' => ['QuoteTransaction', 'Action', 'DefaultStatus'],
+    # Action->Extract Article on a ticket's menu
+    '/Articles/Article/ExtractIntoClass.html' => ['Ticket'],
+);
+
 # Components which are blacklisted from automatic, argument-based whitelisting.
 # These pages are not idempotent when called with just an id.
-our %is_blacklisted_component = (
+our %IS_BLACKLISTED_COMPONENT = (
     # Takes only id and toggles bookmark state
     '/Helpers/Toggle/TicketBookmark' => 1,
 );
@@ -1397,7 +1428,7 @@ sub IsCompCSRFWhitelisted {
     my $comp = shift;
     my $ARGS = shift;
 
-    return 1 if $is_whitelisted_component{$comp};
+    return 1 if $IS_WHITELISTED_COMPONENT{$comp};
 
     my %args = %{ $ARGS };
 
@@ -1417,30 +1448,38 @@ sub IsCompCSRFWhitelisted {
 
     # Some pages aren't idempotent even with safe args like id; blacklist
     # them from the automatic whitelisting below.
-    return 0 if $is_blacklisted_component{$comp};
+    return 0 if $IS_BLACKLISTED_COMPONENT{$comp};
 
-    # 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};
+    if ( my %csrf_config = RT->Config->Get('ReferrerComponents') ) {
+        my $value = $csrf_config{$comp};
+        if ( ref $value eq 'ARRAY' ) {
+            delete $args{$_} for @$value;
+            return %args ? 0 : 1;
+        }
+        else {
+            return $value ? 1 : 0;
+        }
+    }
 
-    # If they have a results= from MaybeRedirectForResults, that's also fine.
-    delete $args{results};
+    return AreCompCSRFParametersWhitelisted($comp, \%args);
+}
 
-    # 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};
+sub AreCompCSRFParametersWhitelisted {
+    my $sub = shift;
+    my $ARGS = shift;
 
-    # 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};
+    my %leftover_args = %{ $ARGS };
+
+    # Join global whitelist and component-specific whitelist
+    my @whitelisted_args = (@GLOBAL_WHITELISTED_ARGS, @{ $WHITELISTED_COMPONENT_ARGS{$sub} || [] });
+
+    for my $arg (@whitelisted_args) {
+        delete $leftover_args{$arg};
+    }
 
     # If there are no arguments, then it's likely to be an idempotent
     # request, which are not susceptible to CSRF
-    return 1 if !%args;
-
-    return 0;
+    return !%leftover_args;
 }
 
 sub IsRefererCSRFWhitelisted {
@@ -3116,8 +3155,9 @@ sub _ParseObjectCustomFieldArgs {
     foreach my $arg ( keys %$ARGSRef ) {
 
         # format: Object-<object class>-<object id>-CustomField[:<grouping>]-<CF id>-<commands>
+        # or: Bulk-<Add or Delete>-CustomField[:<grouping>]-<CF id>-<commands>
         # you can use GetCustomFieldInputName to generate the complement input name
-        next unless $arg =~ /^Object-([\w:]+)-(\d*)-CustomField(?::(\w+))?-(\d+)-(.*)$/;
+        next unless $arg =~ /^(?:Bulk-(?:Add|Delete)|Object-([\w:]+)-(\d*))-CustomField(?::(\w+))?-(\d+)-(.*)$/;
 
         next if $1 eq 'RT::Transaction';# don't try to update transaction fields