XSLoader may load relative paths

[p5pRT]

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

Searchable as RT128528$

[p5pRT]

From @cpansprout

See <https://rt.cpan.org/Ticket/Display.html?id=115808>.  I have already pushed a fix for this as 08e3451. If the current CPAN maintainer of XSLoader is unavailable, then what is our next step? Can someone else make a release?

Should I go ahead and push my fix to maint-5.2x?

[p5pRT]

From @tonycoz

On Sun Jul 03 11​:24​:54 2016, sprout wrote​:

See <https://rt.cpan.org/Ticket/Display.html?id=115808>.  I have
already pushed a fix for this as 08e3451. If the current CPAN
maintainer of XSLoader is unavailable, then what is our next step?
Can someone else make a release?

Should I go ahead and push my fix to maint-5.2x?

+ # Does this look like a relative path?
+ if ($modlibname !~ m|^[\\/]|) {
+ # Someone may have a #line directive that changes the file name, or

Absolute paths might not start with a / or \ on Win32, VMS and (I think) AmigaOS.

Unfortunately loading File​::Spec calls back into XSLoader, so you can't use File​::Spec->file_name_is_absolute

Tony

[p5pRT]

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

[p5pRT]

From @craigberry

On Sun, Jul 3, 2016 at 7​:11 PM, Tony Cook via RT
<perl5-security-report@​perl.org> wrote​:

On Sun Jul 03 11​:24​:54 2016, sprout wrote​:

See <https://rt.cpan.org/Ticket/Display.html?id=115808>.  I have
already pushed a fix for this as 08e3451. If the current CPAN
maintainer of XSLoader is unavailable, then what is our next step?
Can someone else make a release?

Should I go ahead and push my fix to maint-5.2x?

+ # Does this look like a relative path?
+ if ($modlibname !~ m|^[\\/]|) {
+ # Someone may have a #line directive that changes the file name, or

Absolute paths might not start with a / or \ on Win32, VMS and (I think) AmigaOS.

Unfortunately loading File​::Spec calls back into XSLoader, so you can't use File​::Spec->file_name_is_absolute

The package filenames returned by caller() as well as @​INC entries are
stored in Unix format on VMS. If XSLoader​::load is called from
somewhere other than a package, then it's possible to get a filename
from caller() in native syntax, but I'm not sure why anyone would do
that. So I think we're ok there.

On Windows, it looks like both @​INC and the package filenames have
forward slashes, but they do still have the device name at the
beginning of absolute paths, so you have C​:/Perl/lib/... etc. So I
believe relative paths would fail to be identified as such by the
current code.

Do we really have a problem with relative paths as such, or do we have
a problem with package filenames that begin with "(eval" specifically?

[p5pRT]

From @craigberry

On Sun, Jul 3, 2016 at 10​:28 PM, Craig A. Berry <craig.a.berry@​gmail.com> wrote​:

On Windows, it looks like both @​INC and the package filenames have
forward slashes, but they do still have the device name at the
beginning of absolute paths, so you have C​:/Perl/lib/... etc. So I
believe relative paths would fail to be identified as such by the
current code.

I think I said that backwards -- it's absolute paths that will be
considered relative.

[p5pRT]

From @jhi

On Monday-201607-04 3​:11, Tony Cook via RT wrote​:

On Sun Jul 03 11​:24​:54 2016, sprout wrote​:

See <https://rt.cpan.org/Ticket/Display.html?id=115808>.  I have
already pushed a fix for this as 08e3451. If the current CPAN
maintainer of XSLoader is unavailable, then what is our next step?
Can someone else make a release?

Should I go ahead and push my fix to maint-5.2x?

+ # Does this look like a relative path?
+ if ($modlibname !~ m|^[\\/]|) {
+ # Someone may have a #line directive that changes the file name, or

Absolute paths might not start with a / or \ on Win32, VMS and (I think) AmigaOS.

IIUC yes for AmigaOS, it uses the same kind of "multi-root" (volume)
syntax as Win32 or VMS. Would something like m{^\w+​:[/\]} work for
those?

Unfortunately loading File​::Spec calls back into XSLoader, so you
can't use File​::Spec->file_name_is_absolute

Which of course feels kind of stupid. Cwd, I am guessing without
looking. (Or maybe some Win32 things?)

(Cwd being an XS has always been kind of strange, it leads into
silly dependencies. Being core/builtin would make more sense.)

[p5pRT]

From @demerphq

On 4 Jul 2016 02​:19, "Jarkko Hietaniemi" <jhi@​iki.fi> wrote​:

On Monday-201607-04 3​:11, Tony Cook via RT wrote​:

On Sun Jul 03 11​:24​:54 2016, sprout wrote​:

See <https://rt.cpan.org/Ticket/Display.html?id=115808>.  I have
already pushed a fix for this as 08e3451. If the current CPAN
maintainer of XSLoader is unavailable, then what is our next step?
Can someone else make a release?

Should I go ahead and push my fix to maint-5.2x?

+ # Does this look like a relative path?
+ if ($modlibname !~ m|^[\\/]|) {
+ # Someone may have a #line directive that changes the file
name, or

Absolute paths might not start with a / or \ on Win32, VMS and (I think)
AmigaOS.

IIUC yes for AmigaOS, it uses the same kind of "multi-root" (volume)
syntax as Win32 or VMS. Would something like m{^\w+​:[/\]} work for
those?

Unfortunately loading File​::Spec calls back into XSLoader, so you can't
use File​::Spec->file_name_is_absolute

Which of course feels kind of stupid. Cwd, I am guessing without
looking. (Or maybe some Win32 things?)

(Cwd being an XS has always been kind of strange, it leads into
silly dependencies. Being core/builtin would make more sense

++ to that...

[p5pRT]

From @cpansprout

On Jul 4, 2016, at 9​:57 AM, "yves orton via RT" <perl5-security-report@​perl.org> wrote​:

On 4 Jul 2016 02​:19, "Jarkko Hietaniemi" <jhi@​iki.fi> wrote​:

On Monday-201607-04 3​:11, Tony Cook via RT wrote​:

On Sun Jul 03 11​:24​:54 2016, sprout wrote​:

See <https://rt.cpan.org/Ticket/Display.html?id=115808>.  I have
already pushed a fix for this as 08e3451. If the current CPAN
maintainer of XSLoader is unavailable, then what is our next step?
Can someone else make a release?

Should I go ahead and push my fix to maint-5.2x?

+ # Does this look like a relative path?
+ if ($modlibname !~ m|^[\\/]|) {
+ # Someone may have a #line directive that changes the file
name, or

Absolute paths might not start with a / or \ on Win32, VMS and (I think)
AmigaOS.

IIUC yes for AmigaOS, it uses the same kind of "multi-root" (volume)
syntax as Win32 or VMS. Would something like m{^\w+​:[/\]} work for
those?

Unfortunately loading File​::Spec calls back into XSLoader, so you can't
use File​::Spec->file_name_is_absolute

I just looked through all the various implementations of file_name_is_absolute and came up with commit v5.25.2-95-ga651dcd, now pushed to blead. As I pointed out in the commit message, it’s not the end of the world if we mistakenly view some paths as relative, but it will be slightly slower.

[p5pRT]

From @cpansprout

On Jul 3, 2016, at 8​:29 PM, "Craig Berry via RT" <perl5-security-report@​perl.org> wrote​:

Do we really have a problem with relative paths as such, or do we have
a problem with package filenames that begin with "(eval" specifically?

‘(eval’ specifically, but that just reveals the problem. There are numerous reasons why one might use #line directives to change the file name. XSLoader’s documentation does not warn users about it, and, quite frankly, I don’t think users should have to worry about it.

[p5pRT]

From @tonycoz

On Sun, 03 Jul 2016 11​:24​:54 -0700, sprout wrote​:

See <https://rt.cpan.org/Ticket/Display.html?id=115808>.  I have
already pushed a fix for this as 08e3451. If the current CPAN
maintainer of XSLoader is unavailable, then what is our next step?
Can someone else make a release?

Should I go ahead and push my fix to maint-5.2x?

This is fixed in blead and on CPAN - does it need to be fixed anywhere else
to resolve this?

Tony

[p5pRT]

From @iabyn

On Sun, Jan 22, 2017 at 08​:19​:29PM -0800, Tony Cook via RT wrote​:

On Sun, 03 Jul 2016 11​:24​:54 -0700, sprout wrote​:

See <https://rt.cpan.org/Ticket/Display.html?id=115808>.  I have
already pushed a fix for this as 08e3451. If the current CPAN
maintainer of XSLoader is unavailable, then what is our next step?
Can someone else make a release?

Should I go ahead and push my fix to maint-5.2x?

This is fixed in blead and on CPAN - does it need to be fixed anywhere else
to resolve this?

The two XSloader fixes (don't load relative paths, and recognize drive
letters) have been cherry-picked into both maint-5.24 and maint-5.22.
So I think this ticket can be closed.

I also think it can be moved to the public queue, as the XS loader issue
has been public since last July.

--
The warp engines start playing up a bit, but seem to sort themselves out
after a while without any intervention from boy genius Wesley Crusher.
  -- Things That Never Happen in "Star Trek" #17

[p5pRT]

From @iabyn

On Tue, Feb 21, 2017 at 10​:31​:01AM +0000, Dave Mitchell wrote​:

On Sun, Jan 22, 2017 at 08​:19​:29PM -0800, Tony Cook via RT wrote​:

On Sun, 03 Jul 2016 11​:24​:54 -0700, sprout wrote​:

See <https://rt.cpan.org/Ticket/Display.html?id=115808>.  I have
already pushed a fix for this as 08e3451. If the current CPAN
maintainer of XSLoader is unavailable, then what is our next step?
Can someone else make a release?

Should I go ahead and push my fix to maint-5.2x?

This is fixed in blead and on CPAN - does it need to be fixed anywhere else
to resolve this?

The two XSloader fixes (don't load relative paths, and recognize drive
letters) have been cherry-picked into both maint-5.24 and maint-5.22.
So I think this ticket can be closed.

I also think it can be moved to the public queue, as the XS loader issue
has been public since last July.

which I am now doing.

--
That he said that that that that is is is debatable, is debatable.

[p5pRT]

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