xmlparse_setattro() Type Confusion

[JohnLeitch]

BPO	25019
Nosy	@tiran, @serhiy-storchaka
Files	xmlparse_setattro_Type_Confusion.py: repro xmlparse_setattro_Type_Confusion.patch: patch

^{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 = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2015-09-07.20:09:57.923>
created_at = <Date 2015-09-07.15:56:02.238>
labels = ['expert-XML', 'type-crash']
title = 'xmlparse_setattro() Type Confusion'
updated_at = <Date 2015-09-18.12:23:21.777>
user = 'https://bugs.python.org/JohnLeitch'

bugs.python.org fields:

activity = <Date 2015-09-18.12:23:21.777>
actor = 'Arfrever'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-09-07.20:09:57.923>
closer = 'serhiy.storchaka'
components = ['XML']
creation = <Date 2015-09-07.15:56:02.238>
creator = 'JohnLeitch'
dependencies = []
files = ['40394', '40395']
hgrepos = []
issue_num = 25019
keywords = ['patch']
message_count = 3.0
messages = ['250116', '250124', '250126']
nosy_count = 6.0
nosy_names = ['christian.heimes', 'Arfrever', 'python-dev', 'serhiy.storchaka', 'JohnLeitch', 'brycedarling']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue25019'
versions = ['Python 3.4', 'Python 3.5', 'Python 3.6']

[JohnLeitch]

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)

[python-dev]

New changeset ff2c4f281720 by Serhiy Storchaka in branch '3.4':
Issue bpo-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 bpo-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 bpo-25019: Fixed a crash caused by setting non-string key of expat parser.
https://hg.python.org/cpython/rev/edf25acae637

[serhiy-storchaka]

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.