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: xmlparse_setattro() Type Confusion
Type: crash Stage: resolved
Components: XML Versions: Python 3.6, Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Arfrever, JohnLeitch, brycedarling, christian.heimes, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2015-09-07 15:56 by JohnLeitch, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
xmlparse_setattro_Type_Confusion.py JohnLeitch, 2015-09-07 15:56 repro
xmlparse_setattro_Type_Confusion.patch JohnLeitch, 2015-09-07 15:58 patch
Messages (3)
msg250116 - (view) Author: John Leitch (JohnLeitch) * Date: 2015-09-07 15:56
Python 3.4 and 3.5 suffer from a vulnerability caused by the behavior of the xmlparse_setattro() function. When called, the function uses the provided name argument in several conditional statements which assume that the name argument is a string.

However, if a name argument is provided that is not a string, this logic will make several calls to PyUnicode_CompareWithASCIIString that expect a string, yet receive some other type of object, leading to a type confusion vulnerability:

static int
xmlparse_setattro(xmlparseobject *self, PyObject *name, PyObject *v)
{
    /* Set attribute 'name' to value 'v'. v==NULL means delete */
    if (v == NULL) {
        PyErr_SetString(PyExc_RuntimeError, "Cannot delete attribute");
        return -1;
    }
    assert(PyUnicode_Check(name));
    if (PyUnicode_CompareWithASCIIString(name, "buffer_text") == 0) {
    [...]
}
	

In some applications, it may be possible to exploit this behavior to achieve arbitrary code execution. The type confusion can be observed by running the following script:

from xml.parsers.expat import *
p = ParserCreate()
p.__setattr__(range(0xF), 0)

Which, depending on the arrangement of memory, may produce an exception such as this:

0:000> g
(d84.ce0): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
eax=0086f904 ebx=0086f8fc ecx=0050005c edx=00b60138 esi=0050005e edi=00b60138
eip=61e9a967 esp=0086f8c8 ebp=0086f8e0 iopl=0         nv up ei ng nz na pe cy
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010287
python35!find_maxchar_surrogates+0x37:
61e9a967 0fb701          movzx   eax,word ptr [ecx]       ds:002b:0050005c=????
0:000> k3
ChildEBP RetAddr  
0086f8e0 61e9aa35 python35!find_maxchar_surrogates+0x37 [c:\build\cpython\objects\unicodeobject.c @ 1417]
0086f908 61eabcf3 python35!_PyUnicode_Ready+0x35 [c:\build\cpython\objects\unicodeobject.c @ 1466]
0086f918 61c547c3 python35!PyUnicode_CompareWithASCIIString+0x13 
[c:\build\cpython\objects\unicodeobject.c @ 10784]


To fix this issue, it is recommended that xmlparse_setattro() be updated to validate that the name argument is a string and return out of the function early if it is not. A proposed patch is attached.

Credit: John Leitch (johnleitch@outlook.com), Bryce Darling (darlingbryce@gmail.com)
msg250124 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-09-07 19:56
New changeset ff2c4f281720 by Serhiy Storchaka in branch '3.4':
Issue #25019: Fixed a crash caused by setting non-string key of expat parser.
https://hg.python.org/cpython/rev/ff2c4f281720

New changeset 6006231dcaae by Serhiy Storchaka in branch '3.5':
Issue #25019: Fixed a crash caused by setting non-string key of expat parser.
https://hg.python.org/cpython/rev/6006231dcaae

New changeset edf25acae637 by Serhiy Storchaka in branch 'default':
Issue #25019: Fixed a crash caused by setting non-string key of expat parser.
https://hg.python.org/cpython/rev/edf25acae637
msg250126 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-09-07 20:09
Thank you for your contribution John.

The committed patch slightly differs from the proposed patch. Error message now is the same as in setattr() and general __setattr__(). Tests are moved to existing test class for testing of attribute setting. Improved tests for valid attributes.
History
Date User Action Args
2022-04-11 14:58:20adminsetgithub: 69207
2015-09-18 12:23:21Arfreversetnosy: + Arfrever
2015-09-07 20:09:57serhiy.storchakasetstatus: open -> closed
versions: + Python 3.6
type: security -> crash
messages: + msg250126

resolution: fixed
stage: patch review -> resolved
2015-09-07 19:56:27python-devsetnosy: + python-dev
messages: + msg250124
2015-09-07 17:20:40serhiy.storchakasetassignee: serhiy.storchaka

nosy: + serhiy.storchaka
stage: patch review
2015-09-07 16:02:58JohnLeitchsetnosy: + christian.heimes
2015-09-07 16:02:32JohnLeitchsetnosy: + brycedarling
2015-09-07 15:58:55JohnLeitchsetfiles: + xmlparse_setattro_Type_Confusion.patch
keywords: + patch
2015-09-07 15:56:02JohnLeitchcreate