This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: imageop Unsafe Arithmetic
Type: security Stage: resolved
Components: Interpreter Core Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Arfrever, JohnLeitch, python-dev, serhiy.storchaka
Priority: normal Keywords:

Created on 2015-05-22 07:01 by JohnLeitch, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
imageopBufferOverflow.py JohnLeitch, 2015-05-22 07:01 Repro file
Messages (3)
msg243813 - (view) Author: John Leitch (JohnLeitch) * Date: 2015-05-22 07:01
Several functions within the imageop module are vulnerable to exploitable buffer overflows due to unsafe arithmetic in check_multiply_size(). The problem exists because the check to confirm that size == product / y / x does not take remainders into account.

static int
check_multiply_size(int product, int x, const char* xname, int y, const char* yname, int size)
{
    if ( !check_coordonnate(x, xname) )
        return 0;
    if ( !check_coordonnate(y, yname) )
        return 0;
    if ( size == (product / y) / x )
        return 1;
    PyErr_SetString(ImageopError, "String has incorrect length");
    return 0;
}

Consider a call to check_multiply_size() where product is 16, x is 1, and y is 9. In the Windows x86 release of Python 2.7.9, the division is performed using the idiv instruction:

0:000> dV
        product = 0n16
              x = 0n1
          xname = 0x1e1205a4 "x"
              y = 0n9
          yname = 0x1e127ab8 "y"
           size = 0n1
0:000> u eip
python27!check_multiply_size+0x25 [c:\build27\cpython\modules\imageop.c @ 53]:
1e0330e5 f7ff            idiv    eax,edi
1e0330e7 99              cdq
1e0330e8 f7fe            idiv    eax,esi
1e0330ea 3944240c        cmp     dword ptr [esp+0Ch],eax
1e0330ee 7506            jne     python27!check_multiply_size+0x36 (1e0330f6)
1e0330f0 b801000000      mov     eax,1
1e0330f5 c3              ret
1e0330f6 8b15e47e241e    mov     edx,dword ptr [python27!ImageopError (1e247ee4)]
0:000> ?eax
Evaluate expression: 16 = 00000010
0:000> ?edi
Evaluate expression: 9 = 00000009

When the first idiv instruction is executed, the result (eax) is 1 with a remainder of 7 (edx):

0:000> p
eax=00000001 ebx=00000000 ecx=1e127ab8 edx=00000007 esi=00000001 edi=00000009
eip=1e0330e7 esp=0027fcec ebp=00000001 iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00000202
python27!check_multiply_size+0x27:
1e0330e7 99              cdq
0:000> ?eax
Evaluate expression: 1 = 00000001
0:000> ?edx
Evaluate expression: 7 = 00000007

Because size is 1, the check passes:

Breakpoint 4 hit
eax=00000001 ebx=00000000 ecx=1e127ab8 edx=00000000 esi=00000001 edi=00000009
eip=1e0330f0 esp=0027fcec ebp=00000001 iopl=0         nv up ei pl zr na pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00000246
python27!check_multiply_size+0x30:
1e0330f0 b801000000      mov     eax,1

This is problematic because some of the imageop functions, such as grey2rgb, utilize check_multiply_size() to check divisibility prior to copying data into a buffer. Consider a call to grey2rgb where x is 1, y is 9, and len is 16.

static PyObject *
imageop_grey2rgb(PyObject *self, PyObject *args)
{
    int x, y, len, nlen; <<<<<<<< x = 1, y = 9, and len = 16.
    unsigned char *cp;
    unsigned char *ncp;
    PyObject *rv;
    int i;
    unsigned char value;
    int backward_compatible = imageop_backward_compatible();

    if ( !PyArg_ParseTuple(args, "s#ii", &cp, &len, &x, &y) )
        return 0;

    if ( !check_multiply(len, x, y) ) <<<<<<<< 16 != 1 * 9, but this check still passes.
        return 0;
    nlen = x*y*4; <<<<<<<< 1 * 9 * 4 == 36.
    if ( !check_multiply_size(nlen, x, "x", y, "y", 4) )
        return 0;

    rv = PyString_FromStringAndSize(NULL, nlen); <<<<<<<< This creates a buffer of length 36.
    if ( rv == 0 )
        return 0;
    ncp = (unsigned char *)PyString_AsString(rv); <<<<<<<< This retrieves the buffer of length 36.

    for ( i=0; i < len; i++ ) { <<<<<<<< This loop assumes that len * 4 == nlen, which is incorrect
                                         in this case.
        value = *cp++;
        if (backward_compatible) {
            VVVVVVVV Each iteration copies 4 bytes into the 36 byte buffer pointed to by ncp and 
                     advances the pointer by 4. Because len is 16, 64 bytes are ultimately copied
                     into the buffer, leading to an exploitable buffer overflow condition.
            * (Py_UInt32 *) ncp = (Py_UInt32) value | ((Py_UInt32) value << 8 ) | ((Py_UInt32) value << 16);
            ncp += 4;
        } else {
            *ncp++ = 0;
            *ncp++ = value;
            *ncp++ = value;
            *ncp++ = value;
        }
    }
    return rv;
}

When the call completes, memory has been corrupted:

0:000> g
(12f4.640): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=00444444 ebx=00000001 ecx=1e201d98 edx=00303030 esi=1e201d98 edi=1e201d98
eip=1e031fc6 esp=0027fe7c ebp=00000002 iopl=0         nv up ei ng nz ac pe cy
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010297
python27!update_refs+0x6:
1e031fc6 8b5010          mov     edx,dword ptr [eax+10h] ds:002b:00444454=????????
0:000> g
(12f4.640): Access violation - code c0000005 (!!! second chance !!!)
eax=00444444 ebx=00000001 ecx=1e201d98 edx=00303030 esi=1e201d98 edi=1e201d98
eip=1e031fc6 esp=0027fe7c ebp=00000002 iopl=0         nv up ei ng nz ac pe cy
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010297
python27!update_refs+0x6:
1e031fc6 8b5010          mov     edx,dword ptr [eax+10h] ds:002b:00444454=????????

During fuzzing, DEP access violations were encountered, so it should be assumed that this vulnerability can be exploited to achieve arbitrary code execution. To fix this vulnerability, it is recommended that check_multiply_size() confirm divisibility.
msg244518 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-05-31 06:06
New changeset de0ccaaf2e64 by Serhiy Storchaka in branch '2.7':
Issue #24264: Fixed buffer overflow in the imageop module.
https://hg.python.org/cpython/rev/de0ccaaf2e64
msg244519 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-05-31 06:07
Thank you for your report John.
History
Date User Action Args
2022-04-11 14:58:17adminsetgithub: 68452
2015-06-07 13:06:38Arfreversetnosy: + Arfrever
2015-05-31 06:07:16serhiy.storchakasetstatus: open -> closed
messages: + msg244519

assignee: serhiy.storchaka
resolution: fixed
stage: resolved
2015-05-31 06:06:18python-devsetnosy: + python-dev
messages: + msg244518
2015-05-22 08:08:41ned.deilysetnosy: + serhiy.storchaka
2015-05-22 07:01:03JohnLeitchcreate