X-Git-Url: http://git.freeside.biz/gitweb/?p=freeside.git;a=blobdiff_plain;f=rt%2Flib%2FRT%2FInterface%2FWeb.pm;h=f26afde35b920ebcae537d3f65ff621d5fd63a98;hp=dad6a8ee4a2ef4a4816d3afc35c71a2ca1a0cb9a;hb=7322f2afedcc2f427e997d1535a503613a83f088;hpb=ae14e320388fa5e7f400bff1c251ef885b7952e6 diff --git a/rt/lib/RT/Interface/Web.pm b/rt/lib/RT/Interface/Web.pm index dad6a8ee4..f26afde35 100644 --- a/rt/lib/RT/Interface/Web.pm +++ b/rt/lib/RT/Interface/Web.pm @@ -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 # # # (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---CustomField[:]-- + # or: Bulk--CustomField[:]-- # 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