Is this dispatch code insecure?

[Tommy]

I'm worried about $self->can( $user_input ) and what it might allow. Should I maintain a registry of allowed "actions" to which my dispatcher is allowed to route? Or is this good enough? I'm only taking input on AES-encrypted sockets from trusted sources, but in practice... it seems like this could allow a user to call _build_dispatcher for example.

package Foo;
use namespace::autoclean;
use Moose;

has dispatcher => ( is => 'ro', lazy => 1, builder => '_build_dispatch
+er' );

# bare bones dispatcher
sub _build_dispatcher
{
   my $self = shift;

   return sub
   {
      my ( $action, @args ) = @_;

      die "I can't do that, Dave" unless $self->can( $action );

      $self->$action( @args );
   }
}

# ...Elsewhere, in a class that inherits from Foo:
$self->dispatcher->( $action => @params );

I've considered taking queues from Catalyst and using subroutine attributes such that unless a given method has a attribute of :Public ... then I won't allow the call to it. But attributes are ugly right? Hmmmm.

 

[sundialsvc4]

Well, I suppose that any such thing is “as (in)secure as you can make it,” but what you seem to be describing here is fairly similar to what RPC::Any::Server does.   There are many, many ways to do it, and really I think that the only thing which you must be very-sure of is that only intended methods can ultimately be called.   Attributes are certainly one valid way of making such a designation.

I suppose it partly depends on just how many (acceptable) request-types there might be, and how easy it needs to be to add new ones.   Logic that is based on can() and agreed-upon attributes does have the potential advantage that you only need to add code in one place to add new “subroutines” to the system.