classification
Title: io.BufferedRWPair can use uninitialized members
Type: crash Stage: resolved
Components: IO Versions: Python 3.4, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Stephen.Tu, amaury.forgeotdarc, haypo, pitrou, python-dev, serhiy.storchaka
Priority: normal Keywords: needs review, patch

Created on 2013-04-08 23:55 by amaury.forgeotdarc, last changed 2014-02-12 08:57 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
bufferedio.patch Stephen.Tu, 2013-04-13 18:34 review
bufferedio.withtest.patch Stephen.Tu, 2013-04-13 23:03 review
bufferedio_uninitialized.patch serhiy.storchaka, 2014-01-23 21:16 review
Messages (8)
msg186360 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2013-04-08 23:55
This segfaults on all Python versions:
  io.BufferedRWPair.__new__(io.BufferedRWPair).read()

The various "_forward_call" methods should check that the reader and writer objects are correctly initialized. Not NULL, at the very least.
msg186787 - (view) Author: Stephen Tu (Stephen.Tu) * Date: 2013-04-13 18:34
_forward_call() now checks if reader/write is NULL- if so, throws a runtime exception
msg186796 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-04-13 18:50
Thanks for the patch, Stephen. Could you add a test in Lib/test/test_io.py?
msg186865 - (view) Author: Stephen Tu (Stephen.Tu) * Date: 2013-04-13 23:03
patch with test in test_io
msg187028 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-04-15 20:29
About the test:

+        self.assertRaises(Exception, pair.read)
+        self.assertRaises(Exception, pair.write)

First, you should check for the actual RuntimeError.
Second, you need to pass the right arguments for these method calls: for example read(1) and write(b"x"). Otherwise the Exception could correspond to something else.
msg208996 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-23 21:16
Python implementation raises AttributeError,

C implementations of other streams raise ValueError on uninitialized read/write. Therefore I think ValueError is more appropriate for BufferedRWPair too.

Revised patch changes exception type and message to conform with other streams, updates test for uninitialized BufferedRWPair and adds tests for other streams.
msg211064 - (view) Author: Roundup Robot (python-dev) Date: 2014-02-12 08:55
New changeset 712b4665955d by Serhiy Storchaka in branch '2.7':
Issue #17671: Fixed a crash when use non-initialized io.BufferedRWPair.
http://hg.python.org/cpython/rev/712b4665955d

New changeset 25ff4625680d by Serhiy Storchaka in branch '3.3':
Issue #17671: Fixed a crash when use non-initialized io.BufferedRWPair.
http://hg.python.org/cpython/rev/25ff4625680d

New changeset f003ef13c555 by Serhiy Storchaka in branch 'default':
Issue #17671: Fixed a crash when use non-initialized io.BufferedRWPair.
http://hg.python.org/cpython/rev/f003ef13c555
msg211065 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-02-12 08:57
Thank you Stephen for your patch.
History
Date User Action Args
2014-02-12 08:57:19serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg211065

stage: patch review -> resolved
2014-02-12 08:55:36python-devsetnosy: + python-dev
messages: + msg211064
2014-02-10 20:25:50serhiy.storchakasetkeywords: + needs review
assignee: serhiy.storchaka
2014-01-23 21:16:42serhiy.storchakasetfiles: + bufferedio_uninitialized.patch

messages: + msg208996
stage: needs patch -> patch review
2013-04-15 20:29:30pitrousetmessages: + msg187028
2013-04-13 23:03:09Stephen.Tusetfiles: + bufferedio.withtest.patch

messages: + msg186865
2013-04-13 18:50:42pitrousetmessages: + msg186796
2013-04-13 18:34:07Stephen.Tusetfiles: + bufferedio.patch

nosy: + Stephen.Tu
messages: + msg186787

keywords: + patch
2013-04-13 15:02:11serhiy.storchakasetnosy: + serhiy.storchaka

stage: needs patch
2013-04-10 00:33:27hayposetnosy: + haypo
2013-04-08 23:55:32amaury.forgeotdarccreate