search

cpan-authors / IPC-Run

https://metacpan.org/pod/IPC::Run
Other
21 stars 38 forks source link

IPC::Run::run() might run executables that are explicitly added to PATH #162

Open szmate1618 opened 2 years ago

szmate1618 commented 2 years ago

The minimal reproducible example I found is the following (in Powershell):

$Env:PATH="C:\Windows\system32\mspaint.exe"
.\perl.exe -e "use IPC::Run qw(run); my @SomeUndefinedArray = undef; run \@SomeUndefinedArray;"

This will start mspaint.exe, but it could be any executable. The .exe extension can be omitted, as PATHEXT is also taken into account. I believe this is a bug and a potential security issue, as I don't expect run to run anything else than what I pass as an argument. Of course, normally PATH should only contain directories, not executables, but I found it to be fairly easy to accidentally copy-paste a full path to an executable into it. That's how I discovered this behavior.

The root cause of the issue seems to be that passing \@SomeUndefinedArray to run passes an empty string to _search_path which empty string then will be concatenated to every path found in PATH and executed if it points to an executable. Unfortunately every executable path on PATH + an empty string is guaranteed to point to an executable path.

The issue seems to be only present on Windows. I believe the reason for this is the different behavior of File::Spec->catfile() between different OSs: Windows:

> perl.exe -e "use File::Spec; print(File::Spec->catfile('a',''));"
a

WSL terminal:

$ perl -e "use File::Spec; print(File::Spec->catfile('a',''));"
a/

for whatever reason Mac seems to do this:

> perl-e "use File::Spec; print(File::Spec->catfile('a',''));"
a/%

Note the missing trailing separator on Windows.

nmisch commented 2 years ago

Interesting. I agree it's a bug, though I'm not thinking of a way it would harm a program other than a program going out of its way to be harmed. With or without this bug, a program that doesn't trust its PATH needs to replace that PATH with a known-good value. (perlsec documents this.) Even if one misses that precaution, a normal IPC::Run program won't attempt to execute [undef] and will remain unaffected.