homepage

Recently I started doing some research related to language interpreters
and I've stumbled upon a bug in current Python 2.7. I already contacted PSRT and we concluded that this doesn't have security implications.
Repro file looks like this:
```
class Index(object):
 def __index__(self):
 for c in "foobar"*n:
 a.append(c)
 return n * 4
for n in range(1, 100000, 100):
 a = bytearray("test"*n)
 buf = buffer(a)
 s = buf[:Index():1]
```
If you have ASAN build then you'll see this:
```
==29054== ERROR: AddressSanitizer: heap-use-after-free on address 0x60040000a233 at pc 0x4fab7f bp 0x7ffdbfec0b50 sp 0x7ffdbfec0b48
READ of size 1 at 0x60040000a233 thread T0
 #0 0x4fab7e (/home/ad/builds/python-2.7-asan/bin/python2.7+0x4fab7e)
 #1 0x6bbed4 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x6bbed4)
 #2 0x59d998 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x59d998)
 #3 0x5b53fe (/home/ad/builds/python-2.7-asan/bin/python2.7+0x5b53fe)
 #4 0x5b5a65 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x5b5a65)
 #5 0x637eac (/home/ad/builds/python-2.7-asan/bin/python2.7+0x637eac)
 #6 0x63b3af (/home/ad/builds/python-2.7-asan/bin/python2.7+0x63b3af)
 #7 0x4192d0 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x4192d0)
 #8 0x7f6da3cf0f44 (/lib/x86_64-linux-gnu/libc-2.19.so+0x21f44)
 #9 0x417c11 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x417c11)
0x60040000a233 is located 3 bytes inside of 5-byte region [0x60040000a230,0x60040000a235)
freed by thread T0 here:
 #0 0x7f6da49d455f (/usr/lib/x86_64-linux-gnu/libasan.so.0.0.0+0x1555f)
 #1 0x6c5388 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x6c5388)
 #2 0x5b15fb (/home/ad/builds/python-2.7-asan/bin/python2.7+0x5b15fb)
 #3 0x5b53fe (/home/ad/builds/python-2.7-asan/bin/python2.7+0x5b53fe)
 #4 0x6f59c2 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x6f59c2)
 #5 0x440bc8 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x440bc8)
 #6 0x44a712 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x44a712)
 #7 0x440bc8 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x440bc8)
 #8 0x52afeb (/home/ad/builds/python-2.7-asan/bin/python2.7+0x52afeb)
 #9 0x4391ab (/home/ad/builds/python-2.7-asan/bin/python2.7+0x4391ab)
 #10 0x5b5d35 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x5b5d35)
 #11 0x4ea936 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x4ea936)
 #12 0x6bbd20 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x6bbd20)
 #13 0x59d998 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x59d998)
 #14 0x5b53fe (/home/ad/builds/python-2.7-asan/bin/python2.7+0x5b53fe)
 #15 0x5b5a65 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x5b5a65)
 #16 0x637eac (/home/ad/builds/python-2.7-asan/bin/python2.7+0x637eac)
 #17 0x63b3af (/home/ad/builds/python-2.7-asan/bin/python2.7+0x63b3af)
 #18 0x4192d0 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x4192d0)
 #19 0x7f6da3cf0f44 (/lib/x86_64-linux-gnu/libc-2.19.so+0x21f44)
previously allocated by thread T0 here:
 #0 0x7f6da49d455f (/usr/lib/x86_64-linux-gnu/libasan.so.0.0.0+0x1555f)
 #1 0x6c7b3d (/home/ad/builds/python-2.7-asan/bin/python2.7+0x6c7b3d)
 #2 0x6ca853 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x6ca853)
 #3 0x522ddd (/home/ad/builds/python-2.7-asan/bin/python2.7+0x522ddd)
 #4 0x440bc8 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x440bc8)
 #5 0x59f1ca (/home/ad/builds/python-2.7-asan/bin/python2.7+0x59f1ca)
 #6 0x5b53fe (/home/ad/builds/python-2.7-asan/bin/python2.7+0x5b53fe)
 #7 0x5b5a65 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x5b5a65)
 #8 0x637eac (/home/ad/builds/python-2.7-asan/bin/python2.7+0x637eac)
 #9 0x63b3af (/home/ad/builds/python-2.7-asan/bin/python2.7+0x63b3af)
 #10 0x4192d0 (/home/ad/builds/python-2.7-asan/bin/python2.7+0x4192d0)
 #11 0x7f6da3cf0f44 (/lib/x86_64-linux-gnu/libc-2.19.so+0x21f44)
Shadow bytes around the buggy address:
 0x0c00ffff93f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
 0x0c00ffff9400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
 0x0c00ffff9410: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
 0x0c00ffff9420: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
 0x0c00ffff9430: fa fa fa fa fa fa fa fa fa fa fa fa fa fa 00 04
=>0x0c00ffff9440: fa fa fd fa fa fa[fd]fa fa fa fd fa fa fa fd fa
 0x0c00ffff9450: fa fa fd fd fa fa fd fa fa fa fd fa fa fa 00 fa
 0x0c00ffff9460: fa fa 06 fa fa fa fd fa fa fa fd fa fa fa fd fd
 0x0c00ffff9470: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fa
 0x0c00ffff9480: fa fa fd fd fa fa fd fa fa fa 00 fa fa fa fd fa
 0x0c00ffff9490: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa
Shadow byte legend (one shadow byte represents 8 application bytes):
 Addressable: 00
 Partially addressable: 01 02 03 04 05 06 07
 Heap left redzone: fa
 Heap righ redzone: fb
 Freed Heap region: fd
 Stack left redzone: f1
 Stack mid redzone: f2
 Stack right redzone: f3
 Stack partial redzone: f4
 Stack after return: f5
 Stack use after scope: f8
 Global redzone: f9
 Global init order: f6
 Poisoned by user: f7
 ASan internal: fe
==29054== ABORTING
```

The proposed patch fixes this, not sure if a regression test is appropriate here.
Here's a more minimal example that demonstrates the exact problem:
```
class Index():
 def __index__(self):
 global a
 a.append("2")
 return 999
a = bytearray(b"1")
buf = buffer(a)
s = buf[:1:Index()] 
# buf[Index():x:x] or buf[x:x:Index()] will also crash
```
The problem doesn't show up when doing buffer[x:Index()] or [Index():x] because this syntax calls the sq_slice method implemented by buffer object which is passed the indexes as numbers.
However when using slice notation with three arguments, the equivilant of these lines of code is executed:
```
slice_object = slice(x, Index(), x)
buffer[slice_object]
```
During the `buffer[slice_object]`, a call is made in the slice object to find the indexes of the slice. This calls into the __index__ method of the Index class which mutates the underlying storage behind the buffer. However, buffer's subscript method stores the underyling storage in a local variable before calling the GetIndices method (assuming the object won't be mutated) which means that when it returns, it returns a pointer to an older portion of memory.
I took a quick look at listobject, stringobject, unicodeobject, tupleobject and bytearrayobject's subscript methods and it seems they all only access their members after the call to PySlice_GetIndices, so I think they should be fine.
memoryview objects cause a `BufferError: Existing exports of data: object cannot be re-sized` error so Py3 should be fine.

LGTM

Updated patch based on Rietveld review

There a problem with PySlice_GetIndicesEx() (see issue27867). Buffer length shouldn't be evaluated before PySlice_GetIndicesEx() since it can call user code that can change buffer length. This issue can't be solved without first solving issue27867.
get_buf() is called twice. First for getting the size, and later in buffer_item() or after PySlice_GetIndicesEx() for getting a pointer. I think it can be called once.
Ammar, please write a unittest for this issue. It should also cover bugs in the first two versions of the patch.

Proposed patch fixes the issue. But it is hard to write a reliable patch.

New changeset 8cfa6d3065b3 by Serhiy Storchaka in branch '2.7':
Issue #29028: Fixed possible use-after-free bugs in the subscription of the
https://hg.python.org/cpython/rev/8cfa6d3065b3 

Did you forget to close this or is this not fixed, Serhiy?

I wanted first to finish issue27867 (expose new API as public). But this is not needed for this issue.