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

runpy should use io.open_code() instead of open() #82903

Closed
plokmijnuhby mannequin opened this issue Nov 6, 2019 · 12 comments
Closed

runpy should use io.open_code() instead of open() #82903

plokmijnuhby mannequin opened this issue Nov 6, 2019 · 12 comments
Labels
3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-security A security issue

Comments

@plokmijnuhby
Copy link
Mannequin

plokmijnuhby mannequin commented Nov 6, 2019

BPO 38722
Nosy @taleinat, @tiran, @jsnklln, @zooba, @miss-islington, @plokmijnuhby
PRs
  • bpo-38722: Runpy use io.open_code() #17234
  • [3.8] bpo-38722: Runpy use io.open_code() (GH-17234) #17241
  • [3.8] bpo-38722: Runpy use io.open_code() (GH-17234) #17244
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2019-11-18.19:21:16.726>
created_at = <Date 2019-11-06.16:05:25.208>
labels = ['type-security', '3.8', 'library', '3.9']
title = 'runpy should use io.open_code() instead of open()'
updated_at = <Date 2019-11-19.08:33:01.314>
user = 'https://github.com/plokmijnuhby'

    bugs.python.org fields:

    activity = <Date 2019-11-19.08:33:01.314>
actor = 'taleinat'
assignee = 'none'
closed = True
closed_date = <Date 2019-11-18.19:21:16.726>
closer = 'taleinat'
components = ['Library (Lib)']
creation = <Date 2019-11-06.16:05:25.208>
creator = 'plokmijnuhby'
dependencies = []
files = []
hgrepos = []
issue_num = 38722
keywords = ['patch']
message_count = 12.0
messages = ['356144', '356549', '356612', '356877', '356896', '356899', '356900', '356902', '356903', '356904', '356916', '356955']
nosy_count = 6.0
nosy_names = ['taleinat', 'christian.heimes', 'Jason.Killen', 'steve.dower', 'miss-islington', 'plokmijnuhby']
pr_nums = ['17234', '17241', '17244']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue38722'
versions = ['Python 3.8', 'Python 3.9']
    @plokmijnuhby
    Copy link
    Mannequin Author

    plokmijnuhby mannequin commented Nov 6, 2019

    Fairly obviously, if you're using something called runpy you're probably trying to run some code. To do this it has to open the script as a file.

    This is similar to two other issues I'm posting, but they're in different modules, so different bugs.

    @plokmijnuhby plokmijnuhby mannequin added type-security A security issue 3.9 only security fixes stdlib Python modules in the Lib dir labels Nov 6, 2019
    @jsnklln
    Copy link
    Mannequin

    jsnklln mannequin commented Nov 13, 2019

    I'll plan on tackling this one. I already did pdb.

    @jsnklln
    Copy link
    Mannequin

    jsnklln mannequin commented Nov 14, 2019

    I made the change but the test suite is giving me fits and I don't know why.

    Running: ./python -m test
    ...
    == Tests result: FAILURE ==

    392 tests OK.

    1 test failed:
    test_tools

    26 tests skipped:
    test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_idle
    test_kqueue test_msilib test_ossaudiodev test_readline
    test_smtpnet test_socketserver test_startfile test_tcl
    test_timeout test_tix test_tk test_ttk_guionly test_ttk_textonly
    test_turtle test_urllib2net test_urllibnet test_winconsoleio
    test_winreg test_winsound test_xmlrpc_net test_zipfile64

    Total duration: 17 min 38 sec
    Tests result: FAILURE

    But running: ./python -m test -v test_tools
    ...
    Ran 223 tests in 2.503s

    OK (skipped=2, expected failures=14)

    == Tests result: SUCCESS ==

    1 test OK.

    Total duration: 2.6 sec
    Tests result: SUCCESS

    Any tips for a newbe?

    @jsnklln
    Copy link
    Mannequin

    jsnklln mannequin commented Nov 18, 2019

    Tests working now. PR submitted.

    @taleinat taleinat added the 3.8 only security fixes label Nov 18, 2019
    @taleinat
    Copy link
    Contributor

    I don't see why this should be considered a security issue.

    This should likely have been done when io.open_code() was initially added, but now that 3.8 is out, I don't think backporting this would be wise.

    @taleinat taleinat added type-feature A feature request or enhancement and removed 3.8 only security fixes type-security A security issue labels Nov 18, 2019
    @zooba
    Copy link
    Member

    zooba commented Nov 18, 2019

    It's a security issue because Python 3.8 says it will open files to be executed with io.open_code() instead of open(). This allows a way to bypass that.

    That said, this appears to be a fallback case, so I'm not hugely concerned. I haven't quite figured out why it would fall back here (that involved reading the pkgutil sources ;) ).

    I would vote for backporting to 3.8.1, but if Tal wants to push back and nobody else has an opinion then whatever.

    @miss-islington
    Copy link
    Contributor

    New changeset e243bae by Miss Islington (bot) (jsnklln) in branch 'master':
    bpo-38722: Runpy use io.open_code() (GH-17234)
    e243bae

    @taleinat
    Copy link
    Contributor

    Thanks Steve! I hadn't realized that we'd made such a declaration WRT opening of code files in general. In that case, this is certainly at least a bug fix, and should be backported.

    @taleinat taleinat added 3.8 only security fixes type-security A security issue and removed type-feature A feature request or enhancement labels Nov 18, 2019
    @taleinat
    Copy link
    Contributor

    Thanks for reporting this, Dominic!

    Thanks for the PR, Jason!

    @zooba
    Copy link
    Member

    zooba commented Nov 18, 2019

    I hadn't realized that we'd made such a declaration WRT opening of code files in general.

    It wasn't exactly a hugely publicised declaration :)

    The relevant quote from PEP-578 is:

    All import and execution functionality involving code from a file will be changed to use open_code() unconditionally.

    Which I admit is a big claim, and one that was not completely followed through with before 3.8.0. Calling it a "security" fix is borderline, as it isn't really a vulnerability by default, but calling it incorrect behaviour (i.e. a regular bug) is fine by me.

    @miss-islington
    Copy link
    Contributor

    New changeset e37767b by Miss Islington (bot) in branch '3.8':
    bpo-38722: Runpy use io.open_code() (GH-17234)
    e37767b

    @taleinat
    Copy link
    Contributor

    Thanks for the clarification Steve! I've backported this to 3.8.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-security A security issue
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @taleinat @zooba @miss-islington