Skip to content

Commit f56c75e

Browse files
bpo-41288: Fix a crash in unpickling invalid NEWOBJ_EX. (GH-21458)
Automerge-Triggered-By: @tiran (cherry picked from commit 4f309ab) Co-authored-by: Serhiy Storchaka <[email protected]>
1 parent c77f71f commit f56c75e

File tree

3 files changed

+41
-8
lines changed

3 files changed

+41
-8
lines changed

Lib/test/pickletester.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,6 +1170,24 @@ def test_compat_unpickle(self):
11701170
self.assertIs(type(unpickled), collections.UserDict)
11711171
self.assertEqual(unpickled, collections.UserDict({1: 2}))
11721172

1173+
def test_bad_reduce(self):
1174+
self.assertEqual(self.loads(b'cbuiltins\nint\n)R.'), 0)
1175+
self.check_unpickling_error(TypeError, b'N)R.')
1176+
self.check_unpickling_error(TypeError, b'cbuiltins\nint\nNR.')
1177+
1178+
def test_bad_newobj(self):
1179+
error = (pickle.UnpicklingError, TypeError)
1180+
self.assertEqual(self.loads(b'cbuiltins\nint\n)\x81.'), 0)
1181+
self.check_unpickling_error(error, b'cbuiltins\nlen\n)\x81.')
1182+
self.check_unpickling_error(error, b'cbuiltins\nint\nN\x81.')
1183+
1184+
def test_bad_newobj_ex(self):
1185+
error = (pickle.UnpicklingError, TypeError)
1186+
self.assertEqual(self.loads(b'cbuiltins\nint\n)}\x92.'), 0)
1187+
self.check_unpickling_error(error, b'cbuiltins\nlen\n)}\x92.')
1188+
self.check_unpickling_error(error, b'cbuiltins\nint\nN}\x92.')
1189+
self.check_unpickling_error(error, b'cbuiltins\nint\n)N\x92.')
1190+
11731191
def test_bad_stack(self):
11741192
badpickles = [
11751193
b'.', # STOP
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Unpickling invalid NEWOBJ_EX opcode with the C implementation raises now
2+
UnpicklingError instead of crashing.

Modules/_pickle.c

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5988,23 +5988,30 @@ load_newobj_ex(UnpicklerObject *self)
59885988
}
59895989

59905990
if (!PyType_Check(cls)) {
5991-
Py_DECREF(kwargs);
5992-
Py_DECREF(args);
59935991
PyErr_Format(st->UnpicklingError,
59945992
"NEWOBJ_EX class argument must be a type, not %.200s",
59955993
Py_TYPE(cls)->tp_name);
5996-
Py_DECREF(cls);
5997-
return -1;
5994+
goto error;
59985995
}
59995996

60005997
if (((PyTypeObject *)cls)->tp_new == NULL) {
6001-
Py_DECREF(kwargs);
6002-
Py_DECREF(args);
6003-
Py_DECREF(cls);
60045998
PyErr_SetString(st->UnpicklingError,
60055999
"NEWOBJ_EX class argument doesn't have __new__");
6006-
return -1;
6000+
goto error;
6001+
}
6002+
if (!PyTuple_Check(args)) {
6003+
PyErr_Format(st->UnpicklingError,
6004+
"NEWOBJ_EX args argument must be a tuple, not %.200s",
6005+
Py_TYPE(args)->tp_name);
6006+
goto error;
6007+
}
6008+
if (!PyDict_Check(kwargs)) {
6009+
PyErr_Format(st->UnpicklingError,
6010+
"NEWOBJ_EX kwargs argument must be a dict, not %.200s",
6011+
Py_TYPE(kwargs)->tp_name);
6012+
goto error;
60076013
}
6014+
60086015
obj = ((PyTypeObject *)cls)->tp_new((PyTypeObject *)cls, args, kwargs);
60096016
Py_DECREF(kwargs);
60106017
Py_DECREF(args);
@@ -6014,6 +6021,12 @@ load_newobj_ex(UnpicklerObject *self)
60146021
}
60156022
PDATA_PUSH(self->stack, obj, -1);
60166023
return 0;
6024+
6025+
error:
6026+
Py_DECREF(kwargs);
6027+
Py_DECREF(args);
6028+
Py_DECREF(cls);
6029+
return -1;
60176030
}
60186031

60196032
static int

0 commit comments

Comments
 (0)