Skip to content

Commit a52f2cc

Browse files
nwc10iabyn
authored andcommittedMay 10, 2016
Validate the 'require Bare::Word' pathname.
At runtime in require, validate the generated filename after translation of '::' to '/' (and possible conversion from VMS to Unix format) to keep the code simpler. Reject empty module names, module names starting with '/' or '.' (ie absolute paths, hidden files, and '..'), and module names containing NUL bytes or '/.' (ie hidden files and '..'). Add a test for Perl_load_module(), and check that it now rejects module names which fall foul of the above rules. Most of these can't trigger for a sinple bareword require since the illegal module name will already have been rejected during parsing. However, the Perl_load_module() fakes up a rquire optree including a bareword OP_CONST, which *isn't* restricted by the lexer. Note that this doesn't apply to non-bareword pathnames: these are both unaffected: require "/foo/bar.pm"; $x = "/foo/bar.pm"; require $x; [ This is cherry-picked from a branch Nicholas wrote 4 years ago, but which was never merged. I've kept the body of the diff the same, modulo rebasing, but re-worded the commit title and message. Only one test was changed: the final one in load-module.t, since a \0 in a pathname is now trapped earlier and gives a "can't locate" error instead. For the same reason, it also required the addition of "no warnings 'syscalls';". - DAPM ]
1 parent 614273a commit a52f2cc

File tree

8 files changed

+123
-1
lines changed

8 files changed

+123
-1
lines changed
 

‎MANIFEST

+1
Original file line numberDiff line numberDiff line change
@@ -3987,6 +3987,7 @@ ext/XS-APItest/t/labelconst.aux auxiliary file for label test
39873987
ext/XS-APItest/t/labelconst.t test recursive descent label parsing
39883988
ext/XS-APItest/t/labelconst_utf8.aux auxiliary file for label test in UTF-8
39893989
ext/XS-APItest/t/lexsub.t Test XS registration of lexical subs
3990+
ext/XS-APItest/t/load-module.t test load_module()
39903991
ext/XS-APItest/t/locale.t test locale-related things
39913992
ext/XS-APItest/t/loopblock.t test recursive descent block parsing
39923993
ext/XS-APItest/t/looprest.t test recursive descent statement-sequence parsing

‎ext/XS-APItest/APItest.pm

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use strict;
55
use warnings;
66
use Carp;
77

8-
our $VERSION = '0.80';
8+
our $VERSION = '0.81';
99

1010
require XSLoader;
1111

‎ext/XS-APItest/APItest.xs

+12
Original file line numberDiff line numberDiff line change
@@ -4183,6 +4183,18 @@ test_sv_catpvf(SV *fmtsv)
41834183
sv = sv_2mortal(newSVpvn("", 0));
41844184
sv_catpvf(sv, fmt, 5, 6, 7, 8);
41854185

4186+
void
4187+
load_module(flags, name, ...)
4188+
U32 flags
4189+
SV *name
4190+
CODE:
4191+
if (items == 2) {
4192+
Perl_load_module(aTHX_ flags, SvREFCNT_inc(name), NULL);
4193+
} else if (items == 3) {
4194+
Perl_load_module(aTHX_ flags, SvREFCNT_inc(name), SvREFCNT_inc(ST(2)));
4195+
} else
4196+
Perl_croak(aTHX_ "load_module can't yet support %lu items", items);
4197+
41864198
MODULE = XS::APItest PACKAGE = XS::APItest::AUTOLOADtest
41874199

41884200
int

‎ext/XS-APItest/Makefile.PL

+1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ my @names = (qw(HV_DELETE HV_DISABLE_UVAR_XKEY HV_FETCH_ISSTORE
2727
IS_NUMBER_IN_UV IS_NUMBER_GREATER_THAN_UV_MAX
2828
IS_NUMBER_NOT_INT IS_NUMBER_NEG IS_NUMBER_INFINITY
2929
IS_NUMBER_NAN IS_NUMBER_TRAILING PERL_SCAN_TRAILING
30+
PERL_LOADMOD_DENY PERL_LOADMOD_NOIMPORT PERL_LOADMOD_IMPORT_OPS
3031
),
3132
{name=>"G_WANT", default=>["IV", "G_ARRAY|G_VOID"]});
3233

‎ext/XS-APItest/t/load-module.t

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#!perl -w
2+
use strict;
3+
4+
use Test::More;
5+
use XS::APItest;
6+
7+
# This isn't complete yet. In particular, we don't test import lists, or
8+
# the other flags. But it's better than nothing.
9+
10+
is($INC{'less.pm'}, undef, "less isn't loaded");
11+
load_module(PERL_LOADMOD_NOIMPORT, 'less');
12+
like($INC{'less.pm'}, qr!(?:\A|/)lib/less\.pm\z!, "less is now loaded");
13+
14+
delete $INC{'less.pm'};
15+
delete $::{'less::'};
16+
17+
is(eval { load_module(PERL_LOADMOD_NOIMPORT, 'less', 1); 1}, undef,
18+
"expect load_module() to fail");
19+
like($@, qr/less version 1 required--this is only version 0\./,
20+
'with the correct error message');
21+
22+
is(eval { load_module(PERL_LOADMOD_NOIMPORT, 'less', 0.03); 1}, 1,
23+
"expect load_module() not to fail");
24+
25+
for (["", qr!\ABareword in require maps to empty filename!],
26+
["::", qr!\ABareword in require maps to empty filename!],
27+
["::::", qr!\ABareword in require maps to disallowed filename "/\.pm"!],
28+
["::/", qr!\ABareword in require maps to disallowed filename "/\.pm"!],
29+
["::/WOOSH", qr!\ABareword in require maps to disallowed filename "/WOOSH\.pm"!],
30+
[".WOOSH", qr!\ABareword in require maps to disallowed filename "\.WOOSH\.pm"!],
31+
["::.WOOSH", qr!\ABareword in require maps to disallowed filename "\.WOOSH\.pm"!],
32+
["WOOSH::.sock", qr!\ABareword in require contains "/\."!],
33+
["::WOOSH::.sock", qr!\ABareword in require contains "/\."!],
34+
["::WOOSH/.sock", qr!\ABareword in require contains "/\."!],
35+
["::WOOSH/..sock", qr!\ABareword in require contains "/\."!],
36+
["::WOOSH/../sock", qr!\ABareword in require contains "/\."!],
37+
["::WOOSH::..::sock", qr!\ABareword in require contains "/\."!],
38+
["::WOOSH::.::sock", qr!\ABareword in require contains "/\."!],
39+
["::WOOSH::./sock", qr!\ABareword in require contains "/\."!],
40+
["::WOOSH/./sock", qr!\ABareword in require contains "/\."!],
41+
["::WOOSH/.::sock", qr!\ABareword in require contains "/\."!],
42+
["::WOOSH/..::sock", qr!\ABareword in require contains "/\."!],
43+
["::WOOSH::../sock", qr!\ABareword in require contains "/\."!],
44+
["::WOOSH::../..::sock", qr!\ABareword in require contains "/\."!],
45+
["::WOOSH\0sock", qr!\ACan't locate WOOSH\\0sock.pm:!],
46+
) {
47+
my ($module, $error) = @$_;
48+
my $module2 = $module; # load_module mangles its first argument
49+
no warnings 'syscalls';
50+
is(eval { load_module(PERL_LOADMOD_NOIMPORT, $module); 1}, undef,
51+
"expect load_module() for '$module2' to fail");
52+
like($@, $error);
53+
}
54+
55+
done_testing();

‎op.c

+2
Original file line numberDiff line numberDiff line change
@@ -10633,6 +10633,8 @@ Perl_ck_require(pTHX_ OP *o)
1063310633
Move(s+2, s, len - 2, char);
1063410634
end -= 2;
1063510635
}
10636+
if (s == end)
10637+
DIE(aTHX_ "Bareword in require maps to empty filename");
1063610638

1063710639
for (; s < end; s++) {
1063810640
if (*s == ':' && s[1] == ':') {

‎pod/perldiag.pod

+11
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,17 @@ a bareword:
532532

533533
The C<strict> pragma is useful in avoiding such errors.
534534

535+
=item Bareword in require contains "%s"
536+
537+
=item Bareword in require maps to empty filename
538+
539+
=item Bareword in require maps to disallowed filename "%s"
540+
541+
(F) The bareword form of require has been invoked with a filename which could
542+
not have been generated by a valid bareword permitted by the parser. You
543+
shouldn't be able to get this error from Perl code, but XS code may throw it
544+
if it passes an invalid module name to C<Perl_load_module>.
545+
535546
=item Bareword "%s" not allowed while "strict subs" in use
536547

537548
(F) With "strict subs" in use, a bareword is only allowed as a

‎pp_ctl.c

+40
Original file line numberDiff line numberDiff line change
@@ -3727,6 +3727,46 @@ S_require_file(pTHX_ SV *const sv)
37273727
DIE(aTHX_ "Attempt to reload %s aborted.\n"
37283728
"Compilation failed in require", unixname);
37293729
}
3730+
3731+
if (PL_op->op_flags & OPf_KIDS) {
3732+
SVOP * const kid = (SVOP*)cUNOP->op_first;
3733+
3734+
if (kid->op_type == OP_CONST && (kid->op_private & OPpCONST_BARE)) {
3735+
/* require foo (or use foo) with a bareword.
3736+
Perl_load_module fakes up the identical optree, but its
3737+
arguments aren't restricted by the parser to real barewords.
3738+
*/
3739+
const STRLEN package_len = len - 3;
3740+
const char slashdot[2] = {'/', '.'};
3741+
#ifdef DOSISH
3742+
const char backslashdot[2] = {'\\', '.'};
3743+
#endif
3744+
3745+
/* Disallow *purported* barewords that map to absolute
3746+
filenames, filenames relative to the current or parent
3747+
directory, or (*nix) hidden filenames. Also sanity check
3748+
that the generated filename ends .pm */
3749+
if (!path_searchable || len < 3 || name[0] == '.'
3750+
|| !memEQ(name + package_len, ".pm", 3))
3751+
DIE(aTHX_ "Bareword in require maps to disallowed filename \"%"SVf"\"", sv);
3752+
if (memchr(name, 0, package_len)) {
3753+
/* diag_listed_as: Bareword in require contains "%s" */
3754+
DIE(aTHX_ "Bareword in require contains \"\\0\"");
3755+
}
3756+
if (ninstr(name, name + package_len, slashdot,
3757+
slashdot + sizeof(slashdot))) {
3758+
/* diag_listed_as: Bareword in require contains "%s" */
3759+
DIE(aTHX_ "Bareword in require contains \"/.\"");
3760+
}
3761+
#ifdef DOSISH
3762+
if (ninstr(name, name + package_len, backslashdot,
3763+
backslashdot + sizeof(backslashdot))) {
3764+
/* diag_listed_as: Bareword in require contains "%s" */
3765+
DIE(aTHX_ "Bareword in require contains \"\\.\"");
3766+
}
3767+
#endif
3768+
}
3769+
}
37303770
}
37313771

37323772
PERL_DTRACE_PROBE_FILE_LOADING(unixname);

0 commit comments

Comments
 (0)