Skip to content
This repository was archived by the owner on Jun 1, 2023. It is now read-only.

Commit 547b329

Browse files
author
Reini Urban
committed
XSLoader: fix cpan #115808 security
Sync our c code with XSLoader 0.22. Our code already had most of the old .pm problems already solved: (eval \d) is already filtered out with cperl, just relative #line \d filenames could lead to exploits. See https://rt.cpan.org/Ticket/Display.html?id=115808 Make XSLoader fall back to Dynaloader’s @inc search if the calling file has a relative path that is not found in @inc. The filename before auto needs to be absolute and needs to end with a /. Closes #184. See http://perl5.git.perl.org/perl.git/commitdiff/08e3451d7b3b714ad63a27f1b9c2a23ee75d15ee (initial fix) and http://perl5.git.perl.org/perl.git/commitdiff/a651dcdf6a9151150dcf0fb6b18849d3e39b0811 (Windows drive letters as abs) for the perl5 fixes. One other occurance in B::Stash already fixed with 5f8a169.
1 parent f0e5d90 commit 547b329

File tree

4 files changed

+79
-6
lines changed

4 files changed

+79
-6
lines changed

ext/DynaLoader/XSLoader.c

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@
99
* Licensed under the same terms as Perl itself.
1010
*/
1111

12+
#undef WINPATHSEP
13+
#if defined(WIN32) || defined(OS2) || defined(__CYGWIN__) || defined(DOSISH)
14+
|| defined(__SYMBIAN32__) || defined(__amigaos4__)
15+
# define WINPATHSEP
16+
#endif
17+
1218
/* A DynaLoader::bootstrap variant which takes the packagename name from caller() */
1319
XS(XS_XSLoader_load) {
1420
dVAR; dXSARGS;
@@ -47,7 +53,7 @@ XS(XS_XSLoader_load) {
4753
}
4854
if (!modlibname) {
4955
modlibname = OutCopFILE(PL_curcop);
50-
if (memEQ(modlibname, "(eval ", strlen("(eval ")))
56+
if (memEQ(modlibname, "(eval ", 6)) /* This catches RT #115808 */
5157
modlibname = NULL;
5258
}
5359
if (!module) {
@@ -69,22 +75,48 @@ XS(XS_XSLoader_load) {
6975
if (items >= 1) {
7076
SV *caller = newSVpvn_flags(HvNAME(stash), HvNAMELEN(stash), modlibutf8);
7177
modparts = dl_split_modparts(aTHX_ caller);
78+
DLDEBUG(3,PerlIO_printf(Perl_debug_log, " caller %s => '%s'\n",
79+
SvPVX(caller), av_tostr(aTHX_ modparts)));
7280
}
7381
{
7482
SSize_t c = AvFILL(modparts) + 1;
7583
SSize_t i = SvCUR(file);
7684
char *s = SvPVX_mutable(file);
7785
s += i-1;
7886
for (; c>0 && i>=0 && *s; s--, i--) {
79-
if (*s == '/' || *s == '\\') {
87+
if (*s == '/'
88+
#ifdef WINPATHSEP
89+
|| *s == '\\'
90+
#endif
91+
) {
8092
c--;
8193
if (c==0) {
8294
s[1] = 0;
95+
/* ensures ending / */
8396
SvCUR_set(file, i);
8497
break;
8598
}
8699
}
87100
}
101+
if (!SvCUR(file))
102+
goto not_found;
103+
/* must be absolute. see RT #115808 */
104+
s = SvPVX_mutable(file);
105+
if (*s != '/'
106+
#ifdef WINPATHSEP
107+
&& *s != '\\'
108+
&& !(*(s+1) && (*(s+1) == ':') && (*s >= 'A' && *s >= 'Z'))
109+
#endif
110+
)
111+
goto not_found;
112+
s = SvPVX_mutable(file) + SvCUR(file) - 1;
113+
/* and must end with / */
114+
if (*s != '/'
115+
#ifdef WINPATHSEP
116+
&& *s != '\\'
117+
#endif
118+
)
119+
goto not_found;
88120
}
89121
sv_catpv(file, "auto/");
90122
sv_catsv(file, modpname);
@@ -97,6 +129,7 @@ XS(XS_XSLoader_load) {
97129
if (fn_exists(SvPVX(file))) {
98130
DLDEBUG(3,PerlIO_printf(Perl_debug_log, " found '%s'\n", SvPVX(file)));
99131
} else {
132+
not_found:
100133
DLDEBUG(3,PerlIO_printf(Perl_debug_log, " not found '%s'\n", SvPVX(file)));
101134
if (items < 1) {
102135
PUSHMARK(SP);

ext/DynaLoader/dlboot_c.PL

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ print OUT line_prefix(__LINE__, <<'EOT');
149149
newXS("XSLoader::load_file", XS_XSLoader_load_file, xsloaderfile);
150150
newXS("XSLoader::bootstrap_inherit", XS_XSLoader_bootstrap_inherit, xsloaderfile);
151151
/* TODO CM-604: broke t/comp/require.t test 53: require does not localise %^H at run time */
152-
Perl_set_version(aTHX_ STR_WITH_LEN("XSLoader::VERSION"), STR_WITH_LEN("1.01c"), 1.01);
152+
Perl_set_version(aTHX_ STR_WITH_LEN("XSLoader::VERSION"), STR_WITH_LEN("1.02c"), 1.02);
153153
(void)hv_store(inc_hv, "XSLoader.pm", sizeof("XSLoader.pm")-1,
154154
SvREFCNT_inc_simple_NN(newSVpvs("XSLoader.c")), 0);
155155

ext/DynaLoader/t/XSLoader.t

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ my %modules = (
3333
'Time::HiRes'=> q| ::can_ok( 'Time::HiRes' => 'usleep' ) |, # 5.7.3
3434
);
3535

36-
plan tests => keys(%modules) * 3 + 9;
36+
plan tests => keys(%modules) * 3 + 10;
3737

3838
# Try to load the module
3939
use_ok( 'XSLoader' );
@@ -125,3 +125,33 @@ XSLoader::load("Devel::Peek");
125125
EOS
126126
or ::diag $@;
127127
}
128+
129+
SKIP: {
130+
skip "File::Path not available", 1
131+
unless eval { require File::Path };
132+
my $name = "evil$$";
133+
my $fname = "$name/auto/Foo/Bar";
134+
File::Path::mkpath("$name/auto/Foo/Bar");
135+
$fname .= "Bar".$Config::Config{'dlext'};
136+
open my $fh,
137+
">$name/auto/Foo/Bar/Bar.$Config::Config{'dlext'}";
138+
close $fh;
139+
chmod 0755, $fname;
140+
my $fell_back;
141+
local *XSLoader::bootstrap_inherit = sub {
142+
$fell_back++;
143+
# Break out of the calling subs
144+
goto the_test;
145+
};
146+
# https://rt.cpan.org/Ticket/Display.html?id=115808
147+
eval <<END;
148+
#line 1 $name
149+
package Foo::Bar;
150+
XSLoader::load("Foo::Bar");
151+
END
152+
the_test:
153+
ok $fell_back,
154+
'XSLoader will not load relative paths based on (caller)[1]';
155+
sleep(0.2);
156+
File::Path::rmtree($name);
157+
}

pod/perlcdelta.pod

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,13 +194,23 @@ But no classes, methods and roles yet.
194194

195195
=over 4
196196

197-
=item Net-Cmd
197+
=item XSLoader 1.02c
198+
199+
Fixed the C<#line relativefilename> part of the
200+
L<[cpan #115808]|https://rt.cpan.org/Ticket/Display.html?id=115808> security
201+
problem, the C<(eval 1)> part was already fixed in the cperl rewrite as
202+
XS.
203+
204+
Ensure that the local stash filename part is absolute and ends with C</>,
205+
the part before "auto/" and before the fallback to the DynaLoader search.
206+
207+
=item Net::Cmd
198208

199209
Fixed utf8 handling, suse L<[bnc#493978]|https://bugzilla.opensuse.org/show_bug.cgi?id=493978>
200210

201211
Note that libnet has hundreds of more L<open tickets|https://rt.cpan.org/Dist/Display.html?Name=libnet>
202212

203-
=item Pod-Perldoc
213+
=item Pod::Perldoc
204214

205215
Favor nroff over groff, suse perl-nroff.diff [bnc#463444]
206216

0 commit comments

Comments
 (0)