classification
Title: Possible access to unintended variable in "cpython/Objects/sliceobject.c" line 116
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: petrum@gmail.com, serhiy.storchaka, xtreak
Priority: normal Keywords: easy (C), patch

Created on 2018-07-26 04:55 by petrum@gmail.com, last changed 2018-07-26 16:25 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 8480 merged xtreak, 2018-07-26 12:59
Messages (6)
msg322394 - (view) Author: Petru-Florin Mihancea (petrum@gmail.com) Date: 2018-07-26 04:55
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
msg322395 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-26 05:27
Good catch!
msg322405 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018-07-26 07:07
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
msg322406 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-26 07:21
You need to expose it in the _testcapi module as Python function.
msg322436 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-26 16:20
New changeset 2bea7716093012319b5e6a4260fe802b15031f21 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)
https://github.com/python/cpython/commit/2bea7716093012319b5e6a4260fe802b15031f21
msg322437 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-26 16:25
Thank you for your PR Karthikeyan!
History
Date User Action Args
2018-07-26 16:25:53serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg322437

stage: patch review -> resolved
2018-07-26 16:20:36serhiy.storchakasetmessages: + msg322436
2018-07-26 12:59:17xtreaksetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request8003
2018-07-26 07:21:32serhiy.storchakasetmessages: + msg322406
2018-07-26 07:07:01xtreaksetmessages: + msg322405
2018-07-26 05:57:19xtreaksetnosy: + xtreak
2018-07-26 05:27:09serhiy.storchakasettype: behavior
components: + Interpreter Core

keywords: + easy (C)
nosy: + serhiy.storchaka
messages: + msg322395
stage: needs patch
2018-07-26 04:55:28petrum@gmail.comcreate