From 5e90e1438a078bc4fcd07d10ffd0509d6cc45367 Mon Sep 17 00:00:00 2001 From: Ilia Ross Date: Thu, 8 Oct 2026 22:40:36 +0200 Subject: [PATCH] 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. --- t/web-lib-funcs-paths.t | 37 +++++++++++++++++++++++++++++++++++++ web-lib-funcs.pl | 2 +- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/t/web-lib-funcs-paths.t b/t/web-lib-funcs-paths.t index 18e1b5fdc..f45619565 100644 --- a/t/web-lib-funcs-paths.t +++ b/t/web-lib-funcs-paths.t @@ -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: diff --git a/web-lib-funcs.pl b/web-lib-funcs.pl index d01c314c7..84a0686fb 100755 --- a/web-lib-funcs.pl +++ b/web-lib-funcs.pl @@ -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();