Qpsmtpd: untaint config data passed to plugins

if QP passes in tainted data, such as a hostname that subsequently gets used to open a connection using IO::Socket, the plugin die because the information is tainted. Fix it once here, instead of in each plugin.
This commit is contained in:
Matt Simerson 2013-04-25 18:44:21 -04:00
parent c3c5643296
commit 8e9e2926c4

View File

@ -377,25 +377,15 @@ sub _load_plugin {
my $self = shift; my $self = shift;
my ($plugin_line, @plugin_dirs) = @_; my ($plugin_line, @plugin_dirs) = @_;
my ($plugin, @args) = split /\s+/, $plugin_line; # untaint the config data before passing it to plugins
my ($safe_line) = $plugin_line =~ /^([ -~]+)$/ # all ascii printable
my $package; or die "unsafe characters in config line: $plugin_line\n";
my ($plugin, @args) = split /\s+/, $safe_line;
if ($plugin =~ m/::/) { if ($plugin =~ m/::/) {
return $self->_load_package_plugin($plugin, $safe_line, \@args);
};
# "full" package plugin (My::Plugin)
$package = $plugin;
$package =~ s/[^_a-z0-9:]+//gi;
my $eval = qq[require $package;\n]
. qq[sub ${plugin}::plugin_name { '$plugin' }];
$eval =~ m/(.*)/s;
$eval = $1;
eval $eval;
die "Failed loading $package - eval $@" if $@;
$self->log(LOGDEBUG, "Loading $package ($plugin_line)")
unless $plugin_line =~ /logging/;
}
else {
# regular plugins/$plugin plugin # regular plugins/$plugin plugin
my $plugin_name = $plugin; my $plugin_name = $plugin;
$plugin =~ s/:\d+$//; # after this point, only used for filename $plugin =~ s/:\d+$//; # after this point, only used for filename
@ -411,7 +401,7 @@ sub _load_plugin {
"::" . (length $2 ? sprintf("_%2x",unpack("C",$2)) : "") "::" . (length $2 ? sprintf("_%2x",unpack("C",$2)) : "")
]egx; ]egx;
$package = "Qpsmtpd::Plugin::$plugin_name"; my $package = "Qpsmtpd::Plugin::$plugin_name";
# don't reload plugins if they are already loaded # don't reload plugins if they are already loaded
unless (defined &{"${package}::plugin_name"}) { unless (defined &{"${package}::plugin_name"}) {
@ -419,9 +409,8 @@ sub _load_plugin {
if (-e "$dir/$plugin") { if (-e "$dir/$plugin") {
Qpsmtpd::Plugin->compile($plugin_name, $package, Qpsmtpd::Plugin->compile($plugin_name, $package,
"$dir/$plugin", $self->{_test_mode}, $plugin); "$dir/$plugin", $self->{_test_mode}, $plugin);
$self->log(LOGDEBUG, $self->log(LOGDEBUG, "Loading $safe_line from $dir/$plugin")
"Loading $plugin_line from $dir/$plugin") unless $safe_line =~ /logging/;
unless $plugin_line =~ /logging/;
last PLUGIN_DIR; last PLUGIN_DIR;
} }
} }
@ -429,7 +418,6 @@ sub _load_plugin {
join(", ", @plugin_dirs), ")" join(", ", @plugin_dirs), ")"
unless defined &{"${package}::plugin_name"}; unless defined &{"${package}::plugin_name"};
} }
}
my $plug = $package->new(); my $plug = $package->new();
$plug->_register($self, @args); $plug->_register($self, @args);
@ -437,6 +425,26 @@ sub _load_plugin {
return $plug; return $plug;
} }
sub _load_package_plugin {
my ($self, $plugin, $plugin_line, $args) = @_;
# "full" package plugin (My::Plugin)
my $package = $plugin;
$package =~ s/[^_a-z0-9:]+//gi;
my $eval = qq[require $package;\n]
. qq[sub ${plugin}::plugin_name { '$plugin' }];
$eval =~ m/(.*)/s;
$eval = $1;
eval $eval;
die "Failed loading $package - eval $@" if $@;
$self->log(LOGDEBUG, "Loading $package ($plugin_line)")
unless $plugin_line =~ /logging/;
my $plug = $package->new();
$plug->_register($self, @$args);
return $plug;
};
sub transaction { return {}; } # base class implements empty transaction sub transaction { return {}; } # base class implements empty transaction
sub run_hooks { sub run_hooks {