Type confusion in json encoding #68871

pkt · 2015-07-22T07:01:59Z

BPO	24683
Nosy	@rhettinger, @ronaldoussoren, @vstinner, @serhiy-storchaka
Files	json_markers.py json_make_encoder_typecheck.patch eip.py

^{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-07-26.06:10:26.343>
created_at = <Date 2015-07-22.07:01:58.710>
labels = ['extension-modules', 'type-crash']
title = 'Type confusion in json encoding'
updated_at = <Date 2016-02-06.07:10:47.147>
user = 'https://bugs.python.org/pkt'

bugs.python.org fields:

activity = <Date 2016-02-06.07:10:47.147>
actor = 'pkt'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-07-26.06:10:26.343>
closer = 'serhiy.storchaka'
components = ['Extension Modules']
creation = <Date 2015-07-22.07:01:58.710>
creator = 'pkt'
dependencies = []
files = ['39975', '40025', '41719']
hgrepos = []
issue_num = 24683
keywords = ['patch']
message_count = 18.0
messages = ['247093', '247095', '247100', '247111', '247406', '247408', '247410', '247412', '247413', '247454', '247455', '247456', '258950', '258951', '259679', '259681', '259684', '259719']
nosy_count = 7.0
nosy_names = ['rhettinger', 'ronaldoussoren', 'vstinner', 'Arfrever', 'python-dev', 'serhiy.storchaka', 'pkt']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue24683'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']

pkt · 2015-07-22T07:01:58Z

on-35dm-i386-linux-gnu.so`encoder_listencode_list(s=0xb6f90394, acc=0xbfc42c28, seq=0xb6f2361c, indent_level=1) + 655 at _json.c:1800

frame #2: 0xb6e4366d _json.cpython-35dm-i386-linux-gnu.so`encoder_listencode_obj(s=0xb6f90394, acc=0xbfc42c28, obj=0xb6f2361c, indent_level=1) + 733 at _json.c:1554

frame #3: 0xb6e3fc4f _json.cpython-35dm-i386-linux-gnu.so`encoder_call(self=0xb6f90394, args=0xb7049304, kwds=0x00000000) + 319 at _json.c:1386

frame #4: 0x080c5758 python`PyObject_Call(func=0xb6f90394, arg=0xb7049304, kw=0x00000000) + 264 at abstract.c:2149

This is a type confusion bug. encoder->markers can be initialized to an

arbitrary object (string in this POC). PyDict_Contains trusts the caller that

"op" is a dictionary without checking. Some callers can't be trusted :)

vstinner · 2015-07-22T07:22:11Z

I don't understand the issue. Can you elaborate?

What is your code? What is the current result? What is the expected result? What is your platform? What is your Python version? etc.

pkt · 2015-07-22T08:49:04Z

Sorry, I uploaded a test case.

ronaldoussoren · 2015-07-22T11:51:45Z

In encoder_init (the __init__ for _json.Encoder) s->marker is set to an argument of __init__, without any kind of type check, it can therefore be an arbitrary object.

encoder_listencode_obj (and other functions) then use s->markers with the concrete API for dicts (such as PyDict_Contains).

PyDict_Contains does not perform a type check, but casts its first argument to a PyDictObject and access fields. That causes problems when the marker isn't actually a dict.

I don't know the module good enough to be 100% sure about a fix, but I think it would be best to add a type check to encoder_init.

BTW. As far as I know _json.make_encoder is a private API and shouldn't be used directly, when you use the public API the argument will always be a dict.

serhiy-storchaka · 2015-07-26T05:34:49Z

There is similar issue with key_separator and item_separator in 3.x. They are used with _PyAccu_Accumulate that performs a type check only in assert().

Here is a patch.

rhettinger · 2015-07-26T05:45:54Z

Patch LGTM.

python-dev · 2015-07-26T06:04:42Z

New changeset b3d0bf112f70 by Serhiy Storchaka in branch '3.4':
Issue bpo-24683: Fixed crashes in _json functions called with arguments of
https://hg.python.org/cpython/rev/b3d0bf112f70

New changeset ef4d09399b99 by Serhiy Storchaka in branch '3.5':
Issue bpo-24683: Fixed crashes in _json functions called with arguments of
https://hg.python.org/cpython/rev/ef4d09399b99

New changeset 7de4abf4eed2 by Serhiy Storchaka in branch 'default':
Issue bpo-24683: Fixed crashes in _json functions called with arguments of
https://hg.python.org/cpython/rev/7de4abf4eed2

python-dev · 2015-07-26T06:08:55Z

New changeset 0a1266ef1b5d by Serhiy Storchaka in branch '2.7':
Issue bpo-24683: Fixed a crash in _json.make_encoder() called with non-dict 1st argument.
https://hg.python.org/cpython/rev/0a1266ef1b5d

serhiy-storchaka · 2015-07-26T06:10:26Z

Thank you for your report paul. Thanks for review Raymond.

pkt · 2015-07-27T07:38:13Z

resolution: not a bug

^ because of private API?

vstinner · 2015-07-27T07:41:00Z

resolution: not a bug

Since Serhiy pushed changes, the issue is fixed and it is a real bug.

Thanks for the report paul.

serhiy-storchaka · 2015-07-27T08:00:08Z

This was my fault. I want to set resolution to "fixed" but missed. Victor has corrected this.

pkt · 2016-01-26T12:24:08Z

Proof of EIP control.

pkt · 2016-01-26T12:25:03Z

GDB dump of running ./python eip.py

 eax:37A317DD ebx:B7A54268  ecx:BFFFE22C  edx:11223344     eflags:00010217
 esi:B7A61060 edi:B7AA6714  esp:BFFFE20C  ebp:B7A317DC     eip:11223344
 cs:0073  ds:007B  es:007B  fs:0000  gs:0033  ss:007B    o d I t s z A P C

[007B:BFFFE20C]---------------------------------------------------------[stack]
BFFFE23C : 10 FA A1 B7 60 10 A6 B7 - 68 42 A5 B7 00 60 A2 B7 .......hB.....
BFFFE22C : 60 17 A6 B7 10 68 2B 08 - 00 60 A2 B7 DC 17 A3 B7 ....h+........
BFFFE21C : 2C E2 FF BF DC 17 A3 B7 - 3C E2 FF BF 00 00 00 00 ,.......<.......
BFFFE20C : AE 07 0D 08 60 10 A6 B7 - 68 42 A5 B7 DD 17 A3 37 ....`...hB.....7
[0073:11223344]---------------------------------------------------------[ code]
=> 0x11223344: Error while running hook_stop:
Cannot access memory at address 0x11223344
0x11223344 in ?? ()

serhiy-storchaka · 2016-02-05T17:18:53Z

I can't reproduce your example paul.

$ ./python eip.py 
Traceback (most recent call last):
  File "eip.py", line 21, in <module>
    e = j.make_encoder(markers, None, enc, 4, "ks", "is", False, True, True)
TypeError: make_encoder() argument 1 must be dict or None, not array.array

pkt · 2016-02-05T17:39:16Z

Can you try on 2.7 branch?

serhiy-storchaka · 2016-02-05T17:58:46Z

The same result on 2.7 branch:

$ ./python ../cpython/eip.py 
Traceback (most recent call last):
  File "../cpython/eip.py", line 21, in <module>
    e = j.make_encoder(markers, None, enc, 4, "ks", "is", False, True, True)
TypeError: make_encoder() argument 1 must be dict or None, not str

pkt · 2016-02-06T07:10:47Z

Sorry, I wasn't clear enough. This POC is a proof that the original bug can be used for EIP control. I just checked and it works as advertised on 2.7 revision: https://hg.python.org/cpython/rev/2d39777f3477 - it's a parent of https://hg.python.org/cpython/rev/0a1266ef1b5d containing the patch for this issue. I added this file, because I submitted a bug on hackerone claiming EIP control.

pkt mannequin added the type-crash A hard crash of the interpreter, possibly with a core dump label Jul 22, 2015

serhiy-storchaka added the extension-modules C modules in the Modules dir label Jul 22, 2015

serhiy-storchaka self-assigned this Jul 22, 2015

serhiy-storchaka added the invalid label Jul 26, 2015

serhiy-storchaka closed this as completed Jul 26, 2015

vstinner removed the invalid label Jul 27, 2015

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Type confusion in json encoding #68871

Type confusion in json encoding #68871

pkt mannequin commented Jul 22, 2015

pkt mannequin commented Jul 22, 2015

vstinner commented Jul 22, 2015

pkt mannequin commented Jul 22, 2015

ronaldoussoren commented Jul 22, 2015

serhiy-storchaka commented Jul 26, 2015

rhettinger commented Jul 26, 2015

python-dev mannequin commented Jul 26, 2015

python-dev mannequin commented Jul 26, 2015

serhiy-storchaka commented Jul 26, 2015

pkt mannequin commented Jul 27, 2015

vstinner commented Jul 27, 2015

serhiy-storchaka commented Jul 27, 2015

pkt mannequin commented Jan 26, 2016

pkt mannequin commented Jan 26, 2016

serhiy-storchaka commented Feb 5, 2016

pkt mannequin commented Feb 5, 2016

serhiy-storchaka commented Feb 5, 2016

pkt mannequin commented Feb 6, 2016

Type confusion in json encoding #68871

Type confusion in json encoding #68871

Comments

pkt mannequin commented Jul 22, 2015

pkt mannequin commented Jul 22, 2015

frame #2: 0xb6e4366d _json.cpython-35dm-i386-linux-gnu.so`encoder_listencode_obj(s=0xb6f90394, acc=0xbfc42c28, obj=0xb6f2361c, indent_level=1) + 733 at _json.c:1554

frame #3: 0xb6e3fc4f _json.cpython-35dm-i386-linux-gnu.so`encoder_call(self=0xb6f90394, args=0xb7049304, kwds=0x00000000) + 319 at _json.c:1386

frame #4: 0x080c5758 python`PyObject_Call(func=0xb6f90394, arg=0xb7049304, kw=0x00000000) + 264 at abstract.c:2149

This is a type confusion bug. encoder->markers can be initialized to an

arbitrary object (string in this POC). PyDict_Contains trusts the caller that

"op" is a dictionary without checking. Some callers can't be trusted :)

vstinner commented Jul 22, 2015

pkt mannequin commented Jul 22, 2015

ronaldoussoren commented Jul 22, 2015

serhiy-storchaka commented Jul 26, 2015

rhettinger commented Jul 26, 2015

python-dev mannequin commented Jul 26, 2015

python-dev mannequin commented Jul 26, 2015

serhiy-storchaka commented Jul 26, 2015

pkt mannequin commented Jul 27, 2015

vstinner commented Jul 27, 2015

serhiy-storchaka commented Jul 27, 2015

pkt mannequin commented Jan 26, 2016

pkt mannequin commented Jan 26, 2016

serhiy-storchaka commented Feb 5, 2016

pkt mannequin commented Feb 5, 2016

serhiy-storchaka commented Feb 5, 2016

pkt mannequin commented Feb 6, 2016