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

[Dustin Graves]

The branch 4.2/csrf-whitelist was deleted and repushed:
       was de7a29d9e27f37694110f5c8b509275681dc6185
       now 5bdaa0f92f0ff2b4d68fb957b66faa558222aa59

1:  d2c0f0d ! 1:  5bdaa0f add CSRF whitelist for component parameters
    @@ -6,6 +6,7 @@
         but not other parameters
         
         move parameter whitelist logic into own sub
    +    add CSRF tests to test this new behavior
         
         Fixes: I#31090
     
    @@ -17,7 +18,7 @@
      );
      
     +# Whitelist arguments that do not indicate an effectful request.
    -+our @whitelisted_args = (
    ++our @global_whitelisted_args = (
     +    # For example, "id" is acceptable because that is how RT retrieves a
     +    # record.
     +    'id',
    @@ -35,7 +36,9 @@
     +    'NotMobile',
     +);
     +
    -+our %whitelisted_component_parameters = (
    ++our %whitelisted_component_args = (
    ++    # This happens when you middle-(or ⌘ )-click "Edit" for a saved search on
    ++    # the homepage. It's not going to do any damage
     +    '/Search/Build.html' => ['SavedSearchLoad'],
     +);
     +
    @@ -69,12 +72,12 @@
     -    # 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}});
    ++    my @whitelisted_args = (@global_whitelisted_args, @{ $whitelisted_component_args{$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) {
    ++    for my $arg (@whitelisted_args) {
     +        delete $leftover_args{$arg};
     +    }
      
    @@ -84,3 +87,41 @@
      
      sub IsRefererCSRFWhitelisted {
     
    +diff --git a/t/web/csrf.t b/t/web/csrf.t
    +--- a/t/web/csrf.t
    ++++ b/t/web/csrf.t
    +@@
    + $m->content_lacks("Possible cross-site request forgery");
    + $m->title_is('Create a new ticket');
    + 
    ++# CSRF parameter whitelist tests
    ++my $searchBuildPath = '/Search/Build.html';
    ++
    ++# CSRF whitelist for /Search/Build.html param SavedSearchLoad
    ++$m->add_header(Referer => undef);
    ++$m->get_ok("$searchBuildPath?SavedSearchLoad=foo");
    ++$m->content_lacks('Possible cross-site request forgery');
    ++$m->title_is('Query Builder');
    ++
    ++# CSRF pass for /Search/Build.html no param
    ++$m->add_header(Referer => undef);
    ++$m->get_ok("$searchBuildPath");
    ++$m->content_lacks('Possible cross-site request forgery');
    ++$m->title_is('Query Builder');
    ++
    ++# CSRF fail for /Search/Build.html arbitrary param only
    ++$m->add_header(Referer => undef);
    ++$m->get_ok("$searchBuildPath?foo=bar");
    ++$m->content_contains('Possible cross-site request forgery');
    ++$m->title_is('Possible cross-site request forgery');
    ++
    ++# CSRF fail for /Search/Build.html arbitrary param with SavedSearchLoad
    ++$m->add_header(Referer => undef);
    ++$m->get_ok("$searchBuildPath?SavedSearchLoad=foo&foo=bar");
    ++$m->content_contains('Possible cross-site request forgery');
    ++$m->title_is('Possible cross-site request forgery');
    ++
    ++
    + # now send a referer from an attacker
    + $m->add_header(Referer => 'http://example.net');
    + $m->get_ok($test_page);
2:  08a838e < -:  ------- Fixed unsafe array reference lookup with undefined reference
3:  db4f98c < -:  ------- Made expression idiomatic
4:  de7a29d < -:  ------- Added unit tests