[jifty-devel] Re: [Jifty-commit] r4922 - in jifty/trunk: . t/Continuations/lib/Continuations t/TestApp-Plugin-PasswordAuth/t t/TestApp-Plugin-REST/lib/TestApp/Plugin/REST t/TestApp/t

Jesse Vincent jesse at bestpractical.com
Thu Jan 24 11:21:55 EST 2008


This needs an entry in the changelog. And probably for a couple months
at least, an "error" level logged explanation when someone tries to
call a denied action explaining about the change and how to fix it.


On Thu, Jan 24, 2008 at 08:13:54AM -0500, jifty-commit at lists.jifty.org wrote:
> Author: sartak
> Date: Thu Jan 24 08:13:53 2008
> New Revision: 4922
> 
> Modified:
>    jifty/trunk/   (props changed)
>    jifty/trunk/lib/Jifty/API.pm
>    jifty/trunk/lib/Jifty/Dispatcher.pm
>    jifty/trunk/t/Continuations/lib/Continuations/Dispatcher.pm
>    jifty/trunk/t/TestApp-Plugin-PasswordAuth/t/01-tokengen.t
>    jifty/trunk/t/TestApp-Plugin-REST/lib/TestApp/Plugin/REST/Dispatcher.pm
>    jifty/trunk/t/TestApp/t/05-editactions-Cachable.t
>    jifty/trunk/t/TestApp/t/05-editactions-Record.t
> 
> Log:
>  r50714 at onn:  sartak | 2008-01-24 08:13:32 -0500
>  Security fix: Deny all actions (except Autocomplete and Redirect) on GET. You must whitelist actions known to be safe, such as with:
>      before '*' => run { Jifty->api->allow('CustomSearch') };
> 
> 
> Modified: jifty/trunk/lib/Jifty/API.pm
> ==============================================================================
> --- jifty/trunk/lib/Jifty/API.pm	(original)
> +++ jifty/trunk/lib/Jifty/API.pm	Thu Jan 24 08:13:53 2008
> @@ -138,6 +138,24 @@
>      );
>  }
>  
> +=head2 deny_for_get
> +
> +Denies all actions except L<Jifty::Action::Autocomplete> and
> +L<Jifty::Action::Redirect>. This is to protect against a common cross-site
> +scripting hole. In your C<before> dispatcher rules, you can whitelist actions
> +that are known to be read-only.
> +
> +This is called automatically during any C<GET> request.
> +
> +=cut
> +
> +sub deny_for_get {
> +    my $self = shift;
> +    $self->deny(qr/.*/);
> +    $self->allow("Jifty::Action::Autocomplete");
> +    $self->allow("Jifty::Action::Redirect");
> +}
> +
>  =head2 allow RESTRICTIONS
>  
>  Takes a list of strings or regular expressions, and adds them in order
> 
> Modified: jifty/trunk/lib/Jifty/Dispatcher.pm
> ==============================================================================
> --- jifty/trunk/lib/Jifty/Dispatcher.pm	(original)
> +++ jifty/trunk/lib/Jifty/Dispatcher.pm	Thu Jan 24 08:13:53 2008
> @@ -838,6 +838,9 @@
>  
>      $self->log->debug("Dispatching request to ".$self->{path});
>  
> +    # Disable most actions on GET requests
> +    Jifty->api->deny_for_get() if $self->_match_method('GET');
> +
>      # Setup -- we we don't abort out of setup, then run the
>      # actions and then the RUN stage.
>      if ($self->_handle_stage('SETUP')) {
> 
> Modified: jifty/trunk/t/Continuations/lib/Continuations/Dispatcher.pm
> ==============================================================================
> --- jifty/trunk/t/Continuations/lib/Continuations/Dispatcher.pm	(original)
> +++ jifty/trunk/t/Continuations/lib/Continuations/Dispatcher.pm	Thu Jan 24 08:13:53 2008
> @@ -1,6 +1,12 @@
>  package Continuations::Dispatcher;
>  use Jifty::Dispatcher -base;
>  
> +# whitelist these read-only actions
> +before '*' => run {
> +    Jifty->api->allow('GetGrail');
> +    Jifty->api->allow('CrossBridge');
> +};
> +
>  my $before = 0;
>  before '/tutorial' => run {
>      unless (Jifty->web->session->get('got_help')) {
> 
> Modified: jifty/trunk/t/TestApp-Plugin-PasswordAuth/t/01-tokengen.t
> ==============================================================================
> --- jifty/trunk/t/TestApp-Plugin-PasswordAuth/t/01-tokengen.t	(original)
> +++ jifty/trunk/t/TestApp-Plugin-PasswordAuth/t/01-tokengen.t	Thu Jan 24 08:13:53 2008
> @@ -13,7 +13,7 @@
>  use lib 't/lib';
>  use Jifty::SubTest;
>  
> -use Jifty::Test tests => 6;
> +use Jifty::Test tests => 5;
>  use Jifty::Test::WWW::Mechanize;
>  
>  my $server  = Jifty::Test->make_server;
> @@ -26,8 +26,7 @@
>  
>  # {{{ Get token for logging in with a JS-based md5-hashed password
>  my $service='/__jifty/webservices/yaml';
> -my $service_request ="$URL$service?J:A-moniker=GeneratePasswordToken&J:A:F-email-moniker=gooduser\@example.com"; 
> -$mech->get_ok($service_request, "Token-generating webservice $service_request exists");
> +$mech->post("$URL/$service", {"J:A-moniker" => "GeneratePasswordToken", "J:A:F-email-moniker" => 'gooduser at example.com'});
>  
>  # XXX needs to be more precise in checking for the token, but this works
>  # as long as we're using time() for the token
> 
> Modified: jifty/trunk/t/TestApp-Plugin-REST/lib/TestApp/Plugin/REST/Dispatcher.pm
> ==============================================================================
> --- jifty/trunk/t/TestApp-Plugin-REST/lib/TestApp/Plugin/REST/Dispatcher.pm	(original)
> +++ jifty/trunk/t/TestApp-Plugin-REST/lib/TestApp/Plugin/REST/Dispatcher.pm	Thu Jan 24 08:13:53 2008
> @@ -1,4 +1,8 @@
>  package TestApp::Plugin::REST::Dispatcher;
>  use Jifty::Dispatcher -base;
>  
> +before '*' => run {
> +    Jifty->api->allow('DoSomething');
> +};
> +
>  1;
> 
> Modified: jifty/trunk/t/TestApp/t/05-editactions-Cachable.t
> ==============================================================================
> --- jifty/trunk/t/TestApp/t/05-editactions-Cachable.t	(original)
> +++ jifty/trunk/t/TestApp/t/05-editactions-Cachable.t	Thu Jan 24 08:13:53 2008
> @@ -7,7 +7,7 @@
>  use Jifty::SubTest;
>  BEGIN { $ENV{'JIFTY_CONFIG'} = 't/config-Cachable' }
>  
> -use Jifty::Test tests => 8;
> +use Jifty::Test tests => 7;
>  use Jifty::Test::WWW::Mechanize;
>  
>  # Make sure we can load the model
> @@ -31,7 +31,14 @@
>  my $mech    = Jifty::Test::WWW::Mechanize->new();
>  
>  # Test action to update
> -$mech->get_ok($URL.'/editform?J:A-updateuser=TestApp::Action::UpdateUser&J:A:F:F-id-updateuser=1&J:A:F-name-updateuser=edituser&J:A:F-email-updateuser=newemail at example.com', "Form submitted");
> +$mech->post($URL.'/editform', {
> +    'J:A-updateuser' => 'TestApp::Action::UpdateUser',
> +    'J:A:F:F-id-updateuser' => 1,
> +    'J:A:F-name-updateuser' => 'edituser',
> +    'J:A:F-email-updateuser' => 'newemail at example.com',
> +    'J:A:F-tasty-updateuser' => '0'
> +}, "Form submitted");
> +
>  undef $o;
>  $o = TestApp::Model::User->new(current_user => $system_user);
>  $o->flush_cache;
> 
> Modified: jifty/trunk/t/TestApp/t/05-editactions-Record.t
> ==============================================================================
> --- jifty/trunk/t/TestApp/t/05-editactions-Record.t	(original)
> +++ jifty/trunk/t/TestApp/t/05-editactions-Record.t	Thu Jan 24 08:13:53 2008
> @@ -7,7 +7,7 @@
>  use Jifty::SubTest;
>  BEGIN { $ENV{'JIFTY_CONFIG'} = 't/config-Record' }
>  
> -use Jifty::Test tests => 11;
> +use Jifty::Test tests => 10;
>  use Jifty::Test::WWW::Mechanize;
>  # Make sure we can load the model
>  use_ok('TestApp::Model::User');
> @@ -32,7 +32,14 @@
>  my $mech    = Jifty::Test::WWW::Mechanize->new();
>  
>  # Test action to update
> -$mech->get_ok($URL.'/editform?J:A-updateuser=TestApp::Action::UpdateUser&J:A:F:F-id-updateuser=1&J:A:F-name-updateuser=edituser&J:A:F-email-updateuser=newemail at example.com&J:A:F-tasty-updateuser=0', "Form submitted");
> +$mech->post($URL.'/editform', {
> +    'J:A-updateuser' => 'TestApp::Action::UpdateUser',
> +    'J:A:F:F-id-updateuser' => 1,
> +    'J:A:F-name-updateuser' => 'edituser',
> +    'J:A:F-email-updateuser' => 'newemail at example.com',
> +    'J:A:F-tasty-updateuser' => '0'
> +}, "Form submitted");
> +
>  undef $o;
>  $o = TestApp::Model::User->new(current_user => $system_user);
>  $o->load($id);
> _______________________________________________
> Jifty-commit mailing list
> Jifty-commit at lists.jifty.org
> http://lists.jifty.org/cgi-bin/mailman/listinfo/jifty-commit
> 

-- 


More information about the jifty-devel mailing list