Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perl -S erroneously allows \ escapes in PATH #15584

Closed
p5pRT opened this issue Sep 3, 2016 · 15 comments
Closed

perl -S erroneously allows \ escapes in PATH #15584

p5pRT opened this issue Sep 3, 2016 · 15 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 3, 2016

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

Searchable as RT129183$

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2016

From @cpansprout

On Unix, entries in PATH are separated by : and may validly contain backslashes. ‘perl -S’ erroneously

To demonstrate​:

$ mkdir ~/'\'
$ cat > /'\'/foo
#!/usr/bin/perl
print "Hahaha!\n";
^D
$ chmod +x /'\'/foo
$ PATH=
/'\'​:$PATH foo
Hahaha!
$ PATH=
/'\'​:$PATH perl -S foo
Can't find foo on PATH.
$ echo $PATH
/Users/sprout/\​:/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/local/bin

perl is reading the initial ‘/Users/sprout/\​:/usr/bin’ as one PATH entry, which is wrong.

The Perl_find_script function in util.c uses delimcpy to find the colon. delimcpy allows the terminator to be escaped, which is inappropriate for this call site.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2016

From @xenu

On Sat, 3 Sep 2016 12​:24​:34 -0500
"Craig A. Berry" <craig.a.berry@​gmail.com> wrote​:

But isn't colon technically legal (if inadvisable) in a Unix filename?
How would you get one of those in your PATH if you can't escape it?

According to POSIX, it's impossible​:

Since <colon> is a separator in this context, directory names that might
be used in PATH should not include a <colon> character.

http​://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2016

From @cpansprout

On Sat Sep 03 13​:15​:40 2016, me@​xenu.pl wrote​:

On Sat, 3 Sep 2016 12​:24​:34 -0500
"Craig A. Berry" <craig.a.berry@​gmail.com> wrote​:

But isn't colon technically legal (if inadvisable) in a Unix
filename?
How would you get one of those in your PATH if you can't escape it?

According to POSIX, it's impossible​:

Since <colon> is a separator in this context, directory names that
might
be used in PATH should not include a <colon> character.

http​://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03

Indeed, as I demonstrated, if you do try to escape a colon, it will not be escaped. The OS will treat \ as the last character of a path.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2016

From @cpansprout

On Sat Sep 03 13​:20​:14 2016, sprout wrote​:

On Sat Sep 03 13​:15​:40 2016, me@​xenu.pl wrote​:

On Sat, 3 Sep 2016 12​:24​:34 -0500
"Craig A. Berry" <craig.a.berry@​gmail.com> wrote​:

But isn't colon technically legal (if inadvisable) in a Unix
filename?
How would you get one of those in your PATH if you can't escape
it?

According to POSIX, it's impossible​:

Since <colon> is a separator in this context, directory names that
might
be used in PATH should not include a <colon> character.

http​://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03

Indeed, as I demonstrated, if you do try to escape a colon, it will
not be escaped. The OS will treat \ as the last character of a path.

Speaking of which, does VMS allow \ to escape a path separator? Also, if the path separator is sometimes | on VMS, then is the code in util.c​:find_script even correct for VMS?

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2016

From @craigberry

On Sat, Sep 3, 2016 at 3​:20 PM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

On Sat Sep 03 13​:15​:40 2016, me@​xenu.pl wrote​:

On Sat, 3 Sep 2016 12​:24​:34 -0500
"Craig A. Berry" <craig.a.berry@​gmail.com> wrote​:

But isn't colon technically legal (if inadvisable) in a Unix
filename?
How would you get one of those in your PATH if you can't escape it?

According to POSIX, it's impossible​:

Since <colon> is a separator in this context, directory names that
might
be used in PATH should not include a <colon> character.

http​://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03

Indeed, as I demonstrated, if you do try to escape a colon, it will not be escaped. The OS will treat \ as the last character of a path.

Ah, good. I had understood you to be saying that it did escape it and
that's why you got one PATH entry for something that had an escaped
colon in the middle.

@p5pRT
Copy link
Author

p5pRT commented Sep 3, 2016

From @craigberry

On Sat, Sep 3, 2016 at 3​:29 PM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

On Sat Sep 03 13​:20​:14 2016, sprout wrote​:

The OS will treat \ as the last character of a path.

Speaking of which, does VMS allow \ to escape a path separator? Also, if the path separator is sometimes | on VMS, then is the code in util.c​:find_script even correct for VMS?

For purposes of -S, it doesn't look like path separators are involved
at all on VMS. That's because it's not looking in PATH, it's looking
in DCL$PATH, which is a search list logical name, meaning it's an
ordered list that you iterate through, so there is no separator
between elements because the elements are already distinct entries.
Now that you mention it, though, -S is probably doing the wrong thing
for case when running on VMS but under a Unix shell rather than DCL.
I guess I'd have to make all the compile-time checks into run-time
checks in Perl_find_script().

PERL5LIB tries to do both, i.e., function as a search list or as a
single item with separators.

@p5pRT
Copy link
Author

p5pRT commented Oct 11, 2016

From @tonycoz

On Fri Sep 02 22​:28​:05 2016, sprout wrote​:

The Perl_find_script function in util.c uses delimcpy to find the
colon. delimcpy allows the terminator to be escaped, which is
inappropriate for this call site.

So use delimcpy_no_escape()?

Tony

@p5pRT
Copy link
Author

p5pRT commented Oct 11, 2016

From @tonycoz

0001-perl-129183-don-t-treat-as-an-escape-in-PATH-for-S.patch
From a6a25977bac8954bedc8ce17c9429a38535e57a1 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 12 Oct 2016 10:42:47 +1100
Subject: (perl 129183) don't treat \ as an escape in PATH for -S

---
 util.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/util.c b/util.c
index a69ddad..c6727bb 100644
--- a/util.c
+++ b/util.c
@@ -3455,9 +3455,8 @@ Perl_find_script(pTHX_ const char *scriptname, bool dosearch,
 	    if (len < sizeof tmpbuf)
 		tmpbuf[len] = '\0';
 #  else
-	    s = delimcpy(tmpbuf, tmpbuf + sizeof tmpbuf, s, bufend,
-			':',
-			&len);
+	    s = delimcpy_no_escape(tmpbuf, tmpbuf + sizeof tmpbuf, s, bufend,
+                                   ':', &len);
 #  endif
 	    if (s < bufend)
 		s++;
-- 
2.1.4

@p5pRT
Copy link
Author

p5pRT commented Oct 12, 2016

From @cpansprout

On Tue Oct 11 16​:44​:52 2016, tonyc wrote​:

On Fri Sep 02 22​:28​:05 2016, sprout wrote​:

The Perl_find_script function in util.c uses delimcpy to find the
colon. delimcpy allows the terminator to be escaped, which is
inappropriate for this call site.

So use delimcpy_no_escape()?

Yes. (At the time I reported it, I was looking for buggy users of delimcpy, and I correctly surmised that I would not have time to fix them all, hence this bug report.)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Jan 29, 2017

From @craigberry

On Sat, Sep 3, 2016 at 12​:28 AM, Father Chrysostomos
<perlbug-followup@​perl.org> wrote​:

# New Ticket Created by Father Chrysostomos
# Please include the string​: [perl #129183]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=129183 >

On Unix, entries in PATH are separated by : and may validly contain backslashes. ‘perl -S’ erroneously

To demonstrate​:

$ mkdir ~/'\'
$ cat > /'\'/foo
#!/usr/bin/perl
print "Hahaha!\n";
^D
$ chmod +x /'\'/foo
$ PATH=
/'\'​:$PATH foo
Hahaha!
$ PATH=
/'\'​:$PATH perl -S foo
Can't find foo on PATH.
$ echo $PATH
/Users/sprout/\​:/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/local/bin

perl is reading the initial ‘/Users/sprout/\​:/usr/bin’ as one PATH entry, which is wrong.

The Perl_find_script function in util.c uses delimcpy to find the colon. delimcpy allows the terminator to be escaped, which is inappropriate for this call site.

But isn't colon technically legal (if inadvisable) in a Unix filename?
How would you get one of those in your PATH if you can't escape it?

@p5pRT
Copy link
Author

p5pRT commented Jun 14, 2017

From @tonycoz

On Sat, 28 Jan 2017 16​:42​:24 -0800, craig.a.berry@​gmail.com wrote​:

On Sat, Sep 3, 2016 at 12​:28 AM, Father Chrysostomos
<perlbug-followup@​perl.org> wrote​:

The Perl_find_script function in util.c uses delimcpy to find the
colon. delimcpy allows the terminator to be escaped, which is
inappropriate for this call site.

But isn't colon technically legal (if inadvisable) in a Unix filename?
How would you get one of those in your PATH if you can't escape it?

That's covered by the original quote from POSIX​:

directory names that might be used in PATH should not
include a <colon> character.

I've applied my patch as e80af1f

Tony

@p5pRT
Copy link
Author

p5pRT commented Jun 14, 2017

@tonycoz - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented Jun 23, 2018

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release yesterday of Perl 5.28.0, this and 185 other issues have been
resolved.

Perl 5.28.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.28.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT
Copy link
Author

p5pRT commented Jun 23, 2018

@khwilliamson - Status changed from 'pending release' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant