Possible access to unintended variable in "cpython/Objects/sliceobject.c" line 116

[petrufm]

BPO	34229
Nosy	@serhiy-storchaka, @tirkarthi, @petrufm
PRs	bpo-34229: Check start and stop of slice object to be long when they are not int in PySlice_GetIndices #8480

^{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 2018-07-26.16:25:53.567>
created_at = <Date 2018-07-26.04:55:28.685>
labels = ['interpreter-core', 'easy', 'type-bug']
title = 'Possible access to unintended variable in "cpython/Objects/sliceobject.c" line 116'
updated_at = <Date 2018-07-26.16:25:53.566>
user = 'https://github.com/petrufm'

bugs.python.org fields:

activity = <Date 2018-07-26.16:25:53.566>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2018-07-26.16:25:53.567>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2018-07-26.04:55:28.685>
creator = 'petrum@gmail.com'
dependencies = []
files = []
hgrepos = []
issue_num = 34229
keywords = ['patch', 'easy (C)']
message_count = 6.0
messages = ['322394', '322395', '322405', '322406', '322436', '322437']
nosy_count = 3.0
nosy_names = ['serhiy.storchaka', 'xtreak', 'petrum@gmail.com']
pr_nums = ['8480']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue34229'
versions = ['Python 2.7']

[petrufm]

While experimenting with a CodeSonar plugin we develop, we noticed a potential bug in file "cpython/Objects/sliceobject.c" line 116 function PySlice_GetIndices.

if (r->start == Py_None) {
    *start = *step < 0 ? length-1 : 0;
} else {
    if (!PyInt_Check(r->start) && !PyLong_Check(r->step)) return -1;//HERE
    *start = PyInt_AsSsize_t(r->start);
    if (*start < 0) *start += length;
}

Shouldn't start field of r be used in the second check (instead of step)?

In a related potential issue, in line 123, shouldn't r->stop be checked in the second verification?

Thanks,
Petru Florin Mihancea

[serhiy-storchaka]

Good catch!

[tirkarthi]

Is there a way to test this or trigger this code using Python syntax?

slice(start, stop, step).indices() uses slice_indices in Objects/sliceobject.c . I checked the function docs https://docs.python.org/2.7/c-api/slice.html#c.PySlice_GetIndices and it states below

You probably do not want to use this function. If you want to use slice objects in versions of Python prior to 2.3, you would probably do well to incorporate the source of PySlice_GetIndicesEx(), suitably renamed, in the source of your extension.

I couldn't see it's usage too anywhere and with Python 3 we have all integers are implemented as “long” integer objects of arbitrary size and can see only PyLong_Check at https://github.com/python/cpython/blob/master/Objects/sliceobject.c#L178

Thanks

[serhiy-storchaka]

You need to expose it in the _testcapi module as Python function.

[serhiy-storchaka]

New changeset 2bea771 by Serhiy Storchaka (Xtreak) in branch '2.7':
bpo-34229: Check start and stop of slice object to be long when they are not int in PySlice_GetIndices (GH-8480)
2bea771

[serhiy-storchaka]

Thank you for your PR Karthikeyan!