mirror of
https://github.com/webmin/webmin.git
synced 2026-10-21 11:40:40 +01:00
Fix module name validation (once again)
This patch fixes `get_module_info()` accepting module names with a trailing newline after the regex anchors were changed. It preserves the strict validation introduced in my `2f4f017ac` to prevent loading files outside installed modules. The check directly rejects undefined or empty names and any character outside ASCII letters, digits, underscores, and hyphens, including newlines anywhere in the name. Regression tests pass and verify that invalid names are rejected before module lookup while valid names remain accepted.
This commit is contained in:
@@ -17,6 +17,43 @@ my $script = File::Spec->rel2abs(
|
||||
File::Spec->catfile(dirname(__FILE__), '..', 'web-lib-funcs.pl'));
|
||||
require $script;
|
||||
|
||||
# get_module_info — validate names before looking up module files.
|
||||
subtest 'get_module_info name validation' => sub {
|
||||
no warnings qw(redefine once);
|
||||
my @lookups;
|
||||
# Record accepted names and stop before any filesystem access.
|
||||
local *main::module_root_directory = sub {
|
||||
push(@lookups, $_[0]);
|
||||
return '/unused';
|
||||
};
|
||||
local *main::load_language_auto = sub { return 0; };
|
||||
local *main::load_language_neutral = sub { return 0; };
|
||||
local *main::read_file_cached = sub { return 0; };
|
||||
|
||||
# Invalid names must return empty without reaching the module lookup.
|
||||
foreach my $case (
|
||||
[ undef, 'undefined' ], [ '', 'empty' ],
|
||||
[ "module\n", 'trailing newline' ],
|
||||
[ "module\r\n", 'trailing CRLF' ],
|
||||
[ "mod\nule", 'embedded newline' ],
|
||||
[ 'module ', 'trailing space' ],
|
||||
[ "module\t", 'trailing tab' ],
|
||||
[ 'mod.ule', 'dot' ], [ 'mod/ule', 'slash' ],
|
||||
[ "mod\x{e9}ule", 'non-ASCII letter' ]) {
|
||||
@lookups = ();
|
||||
is_deeply([main::get_module_info($case->[0])], [],
|
||||
"$case->[1] returns no module info");
|
||||
is_deeply(\@lookups, [], "$case->[1] skips module lookup");
|
||||
}
|
||||
|
||||
# Valid names, including the string "0", must still reach the lookup.
|
||||
foreach my $name ('module', 'Module_123-test', '0') {
|
||||
@lookups = ();
|
||||
my %info = main::get_module_info($name);
|
||||
is_deeply(\@lookups, [$name], "$name reaches module lookup");
|
||||
}
|
||||
};
|
||||
|
||||
# simplify_path — strip ./ and resolve ../, refusing to escape root.
|
||||
#
|
||||
# Contract:
|
||||
|
||||
@@ -6540,7 +6540,7 @@ sub get_module_info
|
||||
{
|
||||
my ($mod, $noclone, $forcache) = @_;
|
||||
|
||||
return () if (!defined($mod) || $mod !~ /^[A-Za-z0-9_-]+$/);
|
||||
return () if (!defined($mod) || $mod eq "" || $mod =~ /[^A-Za-z0-9_-]/);
|
||||
my (%rv, $clone, $o);
|
||||
my $mdir = &module_root_directory($mod);
|
||||
my $auto = load_language_auto();
|
||||
|
||||
Reference in New Issue
Block a user