better ways than eval to dynamic load a module

jbrugger has asked for the wisdom of the Perl Monks concerning the following question:

Hi fellow monks,

Right now, i use eval() to dynamic load a module. eg:
request: script.pl?option=showOverview&....
in the script i do the following:

my $op = $query->param('option') || 'Login';

if ($op !~ /^[a-z0-9_]*$/i) {
      $op="NotAllowed";
}
eval "require $op";
die "Couldn't find Class $op : ".$@."\n" if $@;
$op->perform(...);
[download]

Now i use a regexp to avoid creative ones from using backticks and other system expressions in their request, however i feel not really secure....

I already thought of a way to create a hash with module names, so the $op is matched as a key value of it's module name, like this:

my $mods = {showOverview=>"showOverview", Login => "Login"}
my $op = $mods->{$query->param('option')};

eval "require $op";
...
[download]

Each time a module is generated, i have to change the script.pl as well however, and i don't like that in 'live' surroundings, (restart http etc).

I hope i'm clear in what i try to accomplish, and hope there are better wayst to accomplish this. Thoughts, ideas and examples are more than welcome.
Thanks in advance (again)

"We all agree on the necessity of compromise. We just can't agree on when it's necessary to compromise." - Larry Wall.

Comment on better ways than eval to dynamic load a module Select or Download Code

Replies are listed 'Best First'.
Re: better ways than eval to dynamic load a module by crashtest (Curate) on Apr 17, 2005 at 19:32 UTC
I understand your reluctance to rely on a regex to catch all the ways a hacker might want to compromise your system. But isn't the expression in your example already too restrictive? `/^[a-z0-9_]*$/i` wouldn't allow files like `Foo.pm`, or even `Foo/Bar.pm`. If you're too uncomfortable to use a regular expression and want to keep a list of allowable modules, you don't have to hard-code that list in your script. Keep the list in a text-file or a database, and query at run-time. Obviously, this would impact performance but if you're dynamically requireing modules, that doesn't appear to be your biggest concern.	[reply] [d/l] [select]
Re^2: better ways than eval to dynamic load a module by adamk (Chaplain) on Apr 18, 2005 at 03:32 UTC
He only needs module names, note the $op->perform. How about something like `use Class::Inspector; my $op = $query->param('option'); unless ( Class::Inspector->installed($op) ) { die "Invalid option '$op'"; } load as normal...` [download]	[reply] [d/l]
Re: better ways than eval to dynamic load a module by etcshadow (Priest) on Apr 17, 2005 at 21:02 UTC
Well, you could always do something like this: `my $file = "$op.pm"; $file =~ s/::/\//g; # / on *nix, \ on windows, etc. the point is: you +r local path separator require $file;` [download] Roughly the same, but no eval. However, if you actually use this regexp on the modulename: `/^[\w:]+$/s`, then I'm pretty certain that it's not possible to put in anything that could be bad. `------------ :Wq Not an editor command: Wq` [download]	[reply] [d/l] [select]
Re: better ways than eval to dynamic load a module by Tanktalus (Canon) on Apr 18, 2005 at 04:14 UTC
IMO, there's a huge difference between eval STR and eval BLOCK. You're using eval STR, and could run into problems with a poor regexp (and I'm not saying you have one). I suggest using eval BLOCK instead. `my $op = $query->param('option') \|\| 'Login'; if ($op =~ /^(\w+)$/) { # this is good for untainting. $op = $1; } else { $op = 'NotAllowed'; } # convert $op into a modulename. (my $modname = $op) =~ s.::./.g; # since you don't allow :'s this isn' +t needed, but it's useful in the general case. $modname .= '.pm'; eval { require $modname }; die "Couldn't find class $op : $@\n" if $@; $op->perform(...);` [download] With your original eval STR code, a carefully crafted option parmaeter, with a broken regular expression (again, yours doesn't seem to be such a case), could insert extra perl commands to run. e.g., an option of "strict;system(qw{rm -rf /})" would be disasterous. You eliminated that with your regexp, but eval BLOCK also eliminates it. IMO, with something so dangerous, it doesn't hurt to double-protect oneself. Just in case you accidentally break your regexp later, for example. (And, as an added bonus, eval BLOCK is faster.)	[reply] [d/l]
Re: better ways than eval to dynamic load a module by jbrugger (Parson) on Apr 18, 2005 at 04:53 UTC
Thanks all for the input. crashtest, i use: `use FindBin; use lib "$FindBin::Bin/PathWithModulesOfMyApp";` [download] so i don't need to have "/" or "\" in $op, that's why the regexp is so strict. The reason to ask, is that imho "require" makes a BIG securiry issue, when the regexp is missing, you'd be able to do things like this `http://127.0.0.1/cgi-bin/script.pl?option=%60touch%20/tmp/HA_I_CAN_WRI +TE_ON_YOUR_HARDDISK.txt%60` [download] and think of more nasty things to do. That's what Tanktalus warns about, and that's why i'm wondering if there is a way to avoid the use of eval totally. I'll have a look at UNIVERSAL::require and Class::Inspector as well, more options are welcome. "We all agree on the necessity of compromise. We just can't agree on when it's necessary to compromise." - Larry Wall.	[reply] [d/l] [select]
Re: better ways than eval to dynamic load a module by cees (Curate) on Apr 18, 2005 at 04:00 UTC
This doesn't answer the important part of your question, but if you want to get rid of the eval, then you could use UNIVERSAL::require to do that. It will make your code look a little bit nicer and more readable without that eval in there. Of course the eval is still done, it is just hidden behind a module. Also, UNIVERSAL::require does not do any validation on the module name, so you will still need to be careful about what you pass in as a module name. So really it won't help you much besides removing the eval from your code ;) - Cees	[reply]