PyUnicode_CopyCharacters could lead to undefined behaviour

[zhangyangyu]

BPO	28379
Nosy	@vstinner, @serhiy-storchaka, @zhangyangyu
PRs	[Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
Files	PyUnicode_CopyCharacters.patch PyUnicode_CopyCharacters_v2.patch unicode_copycharacters.patch

^{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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2016-10-09.12:40:04.925>
created_at = <Date 2016-10-06.18:15:04.479>
labels = ['interpreter-core', 'type-bug', '3.7']
title = 'PyUnicode_CopyCharacters could lead to undefined behaviour'
updated_at = <Date 2017-03-31.16:36:33.183>
user = 'https://github.com/zhangyangyu'

bugs.python.org fields:

activity = <Date 2017-03-31.16:36:33.183>
actor = 'dstufft'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2016-10-09.12:40:04.925>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2016-10-06.18:15:04.479>
creator = 'xiang.zhang'
dependencies = []
files = ['44990', '45001', '45026']
hgrepos = []
issue_num = 28379
keywords = ['patch']
message_count = 10.0
messages = ['278202', '278205', '278240', '278322', '278323', '278342', '278356', '278359', '278373', '278374']
nosy_count = 4.0
nosy_names = ['vstinner', 'python-dev', 'serhiy.storchaka', 'xiang.zhang']
pr_nums = ['552']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue28379'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[zhangyangyu]

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.

[serhiy-storchaka]

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.

[zhangyangyu]

v2 applies Serhiy's suggestions.

[python-dev]

New changeset 13addd71b751 by Serhiy Storchaka in branch '3.5':
Issue bpo-28379: Added sanity checks and tests for PyUnicode_CopyCharacters().
https://hg.python.org/cpython/rev/13addd71b751

[serhiy-storchaka]

Thank you for your contribution.

[zhangyangyu]

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?

[python-dev]

New changeset 1be8cd7cee92 by Serhiy Storchaka in branch 'default':
Issue bpo-28379: Removed redundant check.
https://hg.python.org/cpython/rev/1be8cd7cee92

[zhangyangyu]

We don't need to remove it for 3.5 and 3.6?