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.

Sign up for GitHub

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

Jump to bottom

Add support for handling long shebang lines #4237

Closed
wants to merge 1 commit into from
Closed

Add support for handling long shebang lines #4237

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions pip/wheel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,40 @@ def is_entrypoint_wrapper(name):

maker = ScriptMaker(None, scheme['scripts'])

def get_padded_executable():
executable = sys.executable
if ' ' in executable:
needs_padding = True
else:
platform = sys.platform
if platform.startswith('linux'):
platform = 'linux'
# See http://www.in-ulm.de/~mascheck/various/shebang/#length
shebang_max_lengths = {
'linux': 127,
'darwin': 512,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't see the limits you are using here confirmed in the URL you mention, and there are a lot of other different limits mentioned there. It seems to me that there will be plenty of systems that could still have the issue even with this patch. I can't comment on how obscure those systems might be, of course.

needs_padding = (
len(executable) > shebang_max_lengths.get(platform, 127)
)

if not needs_padding:
return None

# Creates a script that is started as an sh script and then
# re-executed with the specified Python executable. Proper quoting
# makes sure that the same code is valid as both. See
# https://hg.mozilla.org/mozilla-central/file/tip/mach
new_executable = b'/usr/bin/env sh\n'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that /usr/bin/env is not the correct location on every system.

new_executable += b"'''exec' '" + executable + b"'" + b' "$0" "$@"\n'
new_executable += b"' '''"
return new_executable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't an executable name containing ''' break this? I don't know if that could be used to produce an exploit, but it should be considered.

Copy link
Author

@HaraldNordgren HaraldNordgren Jan 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed it does. Not good.


if sys.platform != 'win32':
padded_executable = get_padded_executable()
if padded_executable:
maker.executable = padded_executable

# Ensure old scripts are overwritten.
# See https://github.com/pypa/pip/issues/1800
maker.clobber = True
Expand Down