[Rt-commit] rt branch, 4.2/csrf-whitelist, repushed

[Dustin Graves]

The branch 4.2/csrf-whitelist was deleted and repushed:
       was b7c2c7c5bf43cce02f7fa950d1c58557cc65913e
       now d2c0f0d81b2c3ab2a47c0c66b0c8642b1bf2c5ca

1:  28d3882 < -:  ------- add CSRF whitelist for component parameters
2:  2f6d9a3 ! 1:  d2c0f0d add CSRF whitelist for component parameters
    @@ -1,6 +1,13 @@
     Author: Dustin Graves <dustin at bestpractical.com>
     
    -    move CSRF parameter whitelist logic into own sub
    +    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
     --- a/lib/RT/Interface/Web.pm
    @@ -9,32 +16,32 @@
          '/Ticket/ShowEmailRecord.html' => 1,
      );
      
    --our %is_whitelisted_component_parameter = (
    --    '/Search/Build.html' => { 'SavedSearchLoad' => 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' => 1,
    ++    'id',
     +
     +    # If they have a results= from MaybeRedirectForResults, that's also fine.
    -+    'results' => 1,
    ++    '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' => 1,
    ++    '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' => 1,
    ++    '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 = (
     @@
              }
          }
    @@ -43,7 +50,7 @@
     -    # For example, "id" is acceptable because that is how RT retrieves a
     -    # record.
     -    delete $args{id};
    -+    return AreCompCSRFParametersWhitelisted($comp, $ARGS);
    ++    return AreCompCSRFParametersWhitelisted($comp, \%args);
     +}
      
     -    # If they have a results= from MaybeRedirectForResults, that's also fine.
    @@ -56,18 +63,11 @@
     -    # a referer in most browsers; whitelist the one parameter it reloads
     -    # with, HomeRefreshInterval, which is safe
     -    delete $args{HomeRefreshInterval};
    -+    my %args = %{ $ARGS };
    ++    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};
    -+    my %blacklisted_args = %args;
    - 
    --    # Whitelist compontent parameters
    --    my %is_whitelisted_arg = %{$is_whitelisted_component_parameter{$comp}};
    --    for my $arg (keys %is_whitelisted_arg) {
    --        delete $args{$arg} if $is_whitelisted_arg{$arg};
    --    }
     +    # Join global whitelist and component-specific whitelist
     +    my @comp_whitelisted_args = (@whitelisted_args, @{$whitelisted_component_parameters{$sub}});
      
    @@ -75,12 +75,11 @@
     -    # request, which are not susceptible to CSRF
     -    return 1 if !%args;
     +    for my $arg (@comp_whitelisted_args) {
    -+        delete $blacklisted_args{$arg};
    ++        delete $leftover_args{$arg};
     +    }
      
     -    return 0;
    -+    return !%blacklisted_args;
    ++    return !%leftover_args;
      }
      
      sub IsRefererCSRFWhitelisted {
    -
3:  b7c2c7c < -:  ------- fix argument whitelist array being written with hash syntax