classification
Title: PyUnicode_CopyCharacters could lead to undefined behaviour
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: python-dev, serhiy.storchaka, vstinner, xiang.zhang
Priority: normal Keywords: patch

Created on 2016-10-06 18:15 by xiang.zhang, last changed 2017-03-31 16:36 by dstufft. This issue is now closed.

Files
File name Uploaded Description Edit
PyUnicode_CopyCharacters.patch xiang.zhang, 2016-10-06 18:27 review
PyUnicode_CopyCharacters_v2.patch xiang.zhang, 2016-10-07 13:14 review
unicode_copycharacters.patch xiang.zhang, 2016-10-09 04:41 review
Pull Requests
URL Status Linked Edit
PR 552 closed dstufft, 2017-03-31 16:36
Messages (10)
msg278202 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-10-06 18:15
Currently PyUnicode_CopyCharacters doesn't check arguments thoroughly. This could lead to undefined behaviour or crash in debug mode. For example, from_start > len(from), how_many < 0. Another case is that when how_many > len(from), it will choose len(from) but this can still fail since from_start can > 0. The doc of it is also not perfect, it does not necessarily return 0 on success.
msg278205 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-06 19:08
Added comments on Rietveld. I don't know whether tests for this function are needed. It is public, but is not a part of stable API.
msg278240 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-10-07 13:14
v2 applies Serhiy's suggestions.
msg278322 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-10-08 19:48
New changeset 13addd71b751 by Serhiy Storchaka in branch '3.5':
Issue #28379: Added sanity checks and tests for PyUnicode_CopyCharacters().
https://hg.python.org/cpython/rev/13addd71b751
msg278323 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-08 19:50
Thank you for your contribution.
msg278342 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-10-09 04:41
Thanks Serhiy! But sorry I think I have made a mistake. In unicode_copycharacters we don't need PyUnicode_READY since it has been done in argument parse. Could you remove it?
msg278356 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-10-09 12:38
New changeset 1be8cd7cee92 by Serhiy Storchaka in branch 'default':
Issue #28379: Removed redundant check.
https://hg.python.org/cpython/rev/1be8cd7cee92
msg278359 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-10-09 13:31
We don't need to remove it for 3.5 and 3.6?
msg278373 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-09 16:54
This is just a cleaning up of not very important code.
msg278374 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2016-10-09 17:00
Fine. :-)
History
Date User Action Args
2017-03-31 16:36:33dstufftsetpull_requests: + pull_request1061
2016-10-09 17:00:20xiang.zhangsetmessages: + msg278374
2016-10-09 16:54:19serhiy.storchakasetmessages: + msg278373
2016-10-09 13:31:30xiang.zhangsetmessages: + msg278359
2016-10-09 12:40:04serhiy.storchakasetstatus: open -> closed
stage: patch review -> resolved
2016-10-09 12:38:10python-devsetmessages: + msg278356
2016-10-09 04:41:43xiang.zhangsetstatus: closed -> open
files: + unicode_copycharacters.patch
messages: + msg278342

stage: resolved -> patch review
2016-10-08 19:50:20serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg278323

stage: patch review -> resolved
2016-10-08 19:48:31python-devsetnosy: + python-dev
messages: + msg278322
2016-10-07 20:52:19serhiy.storchakasetassignee: serhiy.storchaka
2016-10-07 13:14:03xiang.zhangsetfiles: + PyUnicode_CopyCharacters_v2.patch

messages: + msg278240
2016-10-06 19:08:42serhiy.storchakasetmessages: + msg278205
2016-10-06 18:27:17xiang.zhangsetfiles: + PyUnicode_CopyCharacters.patch
2016-10-06 18:27:04xiang.zhangsetfiles: - PyUnicode_CopyCharacters.patch
2016-10-06 18:15:04xiang.zhangcreate