require ::foo' will try to load /foo.pm #14861

p5pRT · 2015-08-18T12:22:04Z

Migrated from rt.perl.org#125833 (status was 'resolved')

Searchable as RT125833$

p5pRT · 2015-08-18T12:22:04Z

From @jmdh

As mentioned in

http://www.nntp.perl.org/group/perl.perl5.porters/2012/07/msg189909.html

the bug which was initially described as a security bug is still valid even if not a security bug, and there was quite a bit of discussion about it. However I've not seen a perlbug, nor any recent discussion on the topic, so I'm filing this bug to hopefully provide a definitive record of the status of the issue.

Thanks to all who have contributed to this discussion so far!

Dominic

p5pRT · 2016-01-28T22:54:49Z

From @rjbs

This ticket has fallen off the radar twice, but I am going to get it back on
the radar. *ping*

As mentioned in the ticket, find the original discussion (from 2012!!) here:
http://www.nntp.perl.org/group/perl.perl5.porters/2012/07/msg189909.html

Here's what we should do:

* forbid any leading colons in require or use statements

There is going to be some inconsistency either way, and this is the
inconsistency to pick. I think that FC's suggestion of making "require ::x" a
compile error[1] is a good idea.

Patches solicited.

1: http://www.nntp.perl.org/group/perl.perl5.porters/2012/07/msg189936.html

--
rjbs

p5pRT · 2016-01-28T22:54:49Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2016-02-04T03:36:04Z

From @tonycoz

On Thu Jan 28 14:54:49 2016, perl.p5p@rjbs.manxome.org wrote:

This ticket has fallen off the radar twice, but I am going to get it
back on
the radar. *ping*

As mentioned in the ticket, find the original discussion (from 2012!!)
here:
http://www.nntp.perl.org/group/perl.perl5.porters/2012/07/msg189909.html

Here's what we should do:

* forbid any leading colons in require or use statements

There is going to be some inconsistency either way, and this is the
inconsistency to pick. I think that FC's suggestion of making
"require ::x" a
compile error[1] is a good idea.

Patches solicited.

Attached.

Improvements to the messages and their documentation welcomed.

Tony

p5pRT · 2016-02-04T03:36:04Z

From @tonycoz

0001-perl-125833-disallow-leading-colons-on-require-barew.patch

From ef54951a9db6be28642e783d7d6980d8a96338c3 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Thu, 4 Feb 2016 14:35:03 +1100
Subject: [perl #125833] disallow leading colons on require bareword and use

---
 pod/perldiag.pod | 16 ++++++++++++++++
 t/lib/croak/toke | 12 ++++++++++++
 toke.c           |  4 ++++
 3 files changed, 32 insertions(+)

diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index ba653ba..4c096cd 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -3050,6 +3050,22 @@ L<perlfunc/last>.
 that name, not even if you count where you were called from.  See
 L<perlfunc/last>.
 
+=item Leading colon not permitted for bareword require
+
+(F) Since C<::> in a bareword require is replaced with C</> a require
+of C<::Foo> would attempt to load F</Foo.pm>, which can be surprising.
+
+If you do need to load a module from an absolute path, specify the
+path as a string literal, eg:
+
+  require "/Foo.pm";
+
+=item Leading colon not permitted for use module name
+
+(F) Since C<::> in a module name for C<use> is replaced with C</> a
+C<use ::Foo> would attempt to load F</Foo.pm>, which can be
+surprising.
+
 =item leaving effective %s failed
 
 (F) While under the C<use filetest> pragma, switching the real and
diff --git a/t/lib/croak/toke b/t/lib/croak/toke
index 18dfa24..1b53743 100644
--- a/t/lib/croak/toke
+++ b/t/lib/croak/toke
@@ -302,3 +302,15 @@ Execution of - aborted due to compilation errors.
 BEGIN <>
 EXPECT
 Illegal declaration of subroutine BEGIN at - line 1.
+########
+# NAME use ::Foo [perl #125833]
+use ::Foo;
+EXPECT
+Leading colon not permitted for use module name at - line 1, at end of line
+BEGIN not safe after errors--compilation aborted at - line 1.
+########
+# NAME require ::Foo [perl #125833]
+require ::Foo;
+EXPECT
+Leading colon not permitted for bareword require at - line 1, at end of line
+Execution of - aborted due to compilation errors.
diff --git a/toke.c b/toke.c
index 4a67857..3ca2254 100644
--- a/toke.c
+++ b/toke.c
@@ -4402,6 +4402,8 @@ S_tokenize_use(pTHX_ int is_use, char *s) {
     }
     else {
 	s = force_word(s,WORD,FALSE,TRUE);
+        if (*PL_tokenbuf == ':')
+            yyerror_pv("Leading colon not permitted for use module name", 0);
 	s = force_version(s, FALSE);
     }
     pl_yylval.ival = is_use;
@@ -8020,6 +8022,8 @@ Perl_yylex(pTHX)
                                 GV_ADD | (UTF ? SVf_UTF8 : 0));
 		else if (*s == '<')
 		    yyerror("<> at require-statement should be quotes");
+                if (*PL_tokenbuf == ':')
+                    yyerror_pv("Leading colon not permitted for bareword require", 0);
 	    }
 	    if (orig_keyword == KEY_require) {
 		orig_keyword = 0;
-- 
2.1.4

p5pRT · 2016-02-08T02:25:47Z

From @rjbs

* Tony Cook via RT <perlbug-followup@perl.org> [2016-02-03T22:36:04]

Attached.

Thanks!

This confused me at first:

~/code/perl5$ ./perl -I lib -e 'require :foo'
~/code/perl5$ ./perl -I lib -e 'use :foo'
~/code/perl5$ ./perl -I lib -MO=Deparse,-p -e 'use :foo'
use: '???';
-e syntax OK

Oh, it's a label! Well, no change from existing behavior, so okay! :-)

Improvements to the messages and their documentation welcomed.

They looked good to me.

--
rjbs

p5pRT · 2016-02-08T03:46:50Z

From @ap

* Ricardo Signes <perl.p5p@rjbs.manxome.org> [2016-02-08 03:30]:

* Tony Cook via RT <perlbug-followup@perl.org> [2016-02-03T22:36:04]

Improvements to the messages and their documentation welcomed.

In light of this:

~/code/perl5$ ./perl -I lib -MO=Deparse,-p -e 'use :foo'
use: '???';
-e syntax OK

… maybe reword the error message to “leading double colon”. </bikeshed>

p5pRT · 2016-02-08T23:42:46Z

From @tonycoz

On Sun Feb 07 19:46:50 2016, aristotle wrote:

* Ricardo Signes <perl.p5p@rjbs.manxome.org> [2016-02-08 03:30]:

* Tony Cook via RT <perlbug-followup@perl.org> [2016-02-03T22:36:04]

Improvements to the messages and their documentation welcomed.

In light of this:

~/code/perl5$ ./perl -I lib -MO=Deparse,-p -e 'use :foo'
use: '???';
-e syntax OK

… maybe reword the error message to “leading double colon”. </bikeshed>

Makes sense to me, though I just used '::'.

Also switched to yyerror() instead of yyerror_pv().

Tony

p5pRT · 2016-02-08T23:42:46Z

From @tonycoz

0001-perl-125833-disallow-leading-colons-on-require-barew.patch

From b88180ab9cc77343ec75d0a90014a2e2d4da1cd0 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 9 Feb 2016 10:41:13 +1100
Subject: [perl #125833] disallow leading colons on require bareword and use

---
 pod/perldiag.pod | 16 ++++++++++++++++
 t/lib/croak/toke | 12 ++++++++++++
 toke.c           |  4 ++++
 3 files changed, 32 insertions(+)

diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index ba653ba..3ea45bf 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -3050,6 +3050,22 @@ L<perlfunc/last>.
 that name, not even if you count where you were called from.  See
 L<perlfunc/last>.
 
+=item Leading '::' not permitted for bareword require
+
+(F) Since C<::> in a bareword require is replaced with C</> a require
+of C<::Foo> would attempt to load F</Foo.pm>, which can be surprising.
+
+If you do need to load a module from an absolute path, specify the
+path as a string literal, eg:
+
+  require "/Foo.pm";
+
+=item Leading '::' not permitted for use module name
+
+(F) Since C<::> in a module name for C<use> is replaced with C</> a
+C<use ::Foo> would attempt to load F</Foo.pm>, which can be
+surprising.
+
 =item leaving effective %s failed
 
 (F) While under the C<use filetest> pragma, switching the real and
diff --git a/t/lib/croak/toke b/t/lib/croak/toke
index 18dfa24..7176390 100644
--- a/t/lib/croak/toke
+++ b/t/lib/croak/toke
@@ -302,3 +302,15 @@ Execution of - aborted due to compilation errors.
 BEGIN <>
 EXPECT
 Illegal declaration of subroutine BEGIN at - line 1.
+########
+# NAME use ::Foo [perl #125833]
+use ::Foo;
+EXPECT
+Leading '::' not permitted for use module name at - line 1, at end of line
+BEGIN not safe after errors--compilation aborted at - line 1.
+########
+# NAME require ::Foo [perl #125833]
+require ::Foo;
+EXPECT
+Leading '::' not permitted for bareword require at - line 1, at end of line
+Execution of - aborted due to compilation errors.
diff --git a/toke.c b/toke.c
index 4a67857..185c9da 100644
--- a/toke.c
+++ b/toke.c
@@ -4402,6 +4402,8 @@ S_tokenize_use(pTHX_ int is_use, char *s) {
     }
     else {
 	s = force_word(s,WORD,FALSE,TRUE);
+        if (*PL_tokenbuf == ':')
+            yyerror("Leading '::' not permitted for use module name");
 	s = force_version(s, FALSE);
     }
     pl_yylval.ival = is_use;
@@ -8020,6 +8022,8 @@ Perl_yylex(pTHX)
                                 GV_ADD | (UTF ? SVf_UTF8 : 0));
 		else if (*s == '<')
 		    yyerror("<> at require-statement should be quotes");
+                if (*PL_tokenbuf == ':')
+                    yyerror("Leading '::' not permitted for bareword require");
 	    }
 	    if (orig_keyword == KEY_require) {
 		orig_keyword = 0;
-- 
2.1.4

p5pRT · 2016-03-25T02:08:34Z

From @rjbs

I've added this to the v5.25.1 blockers. Dave M. proposes an alternate patch in nntp.perl.org/group/perl.perl5.porters/235248

--
rjbs

p5pRT · 2016-05-10T10:22:30Z

From @iabyn

On Thu, Mar 24, 2016 at 07:08:35PM -0700, Ricardo SIGNES via RT wrote:

I've added this to the v5.25.1 blockers. Dave M. proposes an alternate patch in nntp.perl.org/group/perl.perl5.porters/235248

Now in blead as

8135bae [MERGE] disallow 'require ::Foo::Bar' etc
09cfff4 load-module.t: re-indent and add some comments.
5bad2b3 make 'require ::Foo::Bar' die
a52f2cc Validate the 'require Bare::Word' pathname.
614273a Treat require ::foo::bar; the same as foo::bar;
9cdec13 reindent S_require_version()
5fb4138 Split the guts of pp_require into two static fns

--
The Enterprise is captured by a vastly superior alien intelligence which
does not put them on trial.
-- Things That Never Happen in "Star Trek" #10

p5pRT · 2016-05-14T10:35:55Z

@iabyn - Status changed from 'open' to 'resolved'

p5pRT closed this as completed May 14, 2016

p5pRT added the Severity Low label Oct 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

require ::foo' will try to load /foo.pm #14861

require ::foo' will try to load /foo.pm #14861

p5pRT commented Aug 18, 2015

p5pRT commented Aug 18, 2015

p5pRT commented Jan 28, 2016

p5pRT commented Jan 28, 2016

p5pRT commented Feb 4, 2016

p5pRT commented Feb 4, 2016

p5pRT commented Feb 8, 2016

p5pRT commented Feb 8, 2016

p5pRT commented Feb 8, 2016

p5pRT commented Feb 8, 2016

p5pRT commented Mar 25, 2016

p5pRT commented May 10, 2016

p5pRT commented May 14, 2016

require ::foo' will try to load /foo.pm #14861

require ::foo' will try to load /foo.pm #14861

Comments

p5pRT commented Aug 18, 2015

p5pRT commented Aug 18, 2015

From @jmdh

p5pRT commented Jan 28, 2016

From @rjbs

p5pRT commented Jan 28, 2016

p5pRT commented Feb 4, 2016

From @tonycoz

p5pRT commented Feb 4, 2016

From @tonycoz

p5pRT commented Feb 8, 2016

From @rjbs

p5pRT commented Feb 8, 2016

From @ap

p5pRT commented Feb 8, 2016

From @tonycoz

p5pRT commented Feb 8, 2016

From @tonycoz

p5pRT commented Mar 25, 2016

From @rjbs

p5pRT commented May 10, 2016

From @iabyn

p5pRT commented May 14, 2016