Title: runpy should use io.open_code() instead of open()
Type: security Stage: resolved
Components: Library (Lib) Versions: Python 3.9, Python 3.8
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Jason.Killen, christian.heimes, miss-islington, plokmijnuhby, steve.dower, taleinat
Priority: normal Keywords: patch

Created on 2019-11-06 16:05 by plokmijnuhby, last changed 2019-11-19 08:33 by taleinat. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 17234 merged Jason.Killen, 2019-11-18 16:14
PR 17241 closed miss-islington, 2019-11-18 19:17
PR 17244 merged miss-islington, 2019-11-18 21:29
Messages (12)
msg356144 - (view) Author: Dominic Littlewood (plokmijnuhby) * Date: 2019-11-06 16:05
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.
msg356549 - (view) Author: Jason Killen (Jason.Killen) * Date: 2019-11-13 19:33
I'll plan on tackling this one.  I already did pdb.
msg356612 - (view) Author: Jason Killen (Jason.Killen) * Date: 2019-11-14 15:37
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:

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?
msg356877 - (view) Author: Jason Killen (Jason.Killen) * Date: 2019-11-18 16:18
Tests working now.  PR submitted.
msg356896 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-11-18 18:48
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.
msg356899 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-18 19:10
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.
msg356900 - (view) Author: miss-islington (miss-islington) Date: 2019-11-18 19:11
New changeset e243bae9999418859106328d9fce71815b7eb2fe by Miss Islington (bot) (jsnklln) in branch 'master':
bpo-38722: Runpy use io.open_code() (GH-17234)
msg356902 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-11-18 19:16
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.
msg356903 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-11-18 19:21
Thanks for reporting this, Dominic!

Thanks for the PR, Jason!
msg356904 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-11-18 19:32
> 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.
msg356916 - (view) Author: miss-islington (miss-islington) Date: 2019-11-18 21:58
New changeset e37767bee1f7f1940b30768d21bfe2ae68c20a5f by Miss Islington (bot) in branch '3.8':
bpo-38722: Runpy use io.open_code() (GH-17234)
msg356955 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2019-11-19 08:33
Thanks for the clarification Steve! I've backported this to 3.8.
Date User Action Args
2019-11-19 08:33:01taleinatsetmessages: + msg356955
2019-11-18 21:58:06miss-islingtonsetmessages: + msg356916
2019-11-18 21:29:41miss-islingtonsetpull_requests: + pull_request16743
2019-11-18 19:32:08steve.dowersetmessages: + msg356904
2019-11-18 19:21:16taleinatsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-11-18 19:21:04taleinatsetmessages: + msg356903
2019-11-18 19:17:04miss-islingtonsetpull_requests: + pull_request16740
2019-11-18 19:16:29taleinatsettype: enhancement -> security
messages: + msg356902
versions: + Python 3.8
2019-11-18 19:11:21miss-islingtonsetnosy: + miss-islington
messages: + msg356900
2019-11-18 19:10:40steve.dowersetnosy: + christian.heimes
messages: + msg356899
2019-11-18 18:57:48zach.waresetnosy: + steve.dower
2019-11-18 18:48:24taleinatsetversions: - Python 3.8
nosy: + taleinat

messages: + msg356896

type: security -> enhancement
2019-11-18 17:40:40taleinatsetversions: + Python 3.8
2019-11-18 16:18:53Jason.Killensetmessages: + msg356877
2019-11-18 16:14:56Jason.Killensetkeywords: + patch
stage: patch review
pull_requests: + pull_request16734
2019-11-14 15:37:55Jason.Killensetmessages: + msg356612
2019-11-13 19:33:48Jason.Killensetnosy: + Jason.Killen
messages: + msg356549
2019-11-06 16:05:25plokmijnuhbycreate