Skip to content

Commit

Permalink
bpo-41288: Fix a crash in unpickling invalid NEWOBJ_EX. (pythonGH-21458)
Browse files Browse the repository at this point in the history
Automerge-Triggered-By: @tiran
  • Loading branch information
serhiy-storchaka authored Jul 13, 2020
1 parent b0689ae commit 4f309ab
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 8 deletions.
18 changes: 18 additions & 0 deletions Lib/test/pickletester.py
Original file line number Diff line number Diff line change
Expand Up @@ -1176,6 +1176,24 @@ def test_compat_unpickle(self):
self.assertIs(type(unpickled), collections.UserDict)
self.assertEqual(unpickled, collections.UserDict({1: 2}))

def test_bad_reduce(self):
self.assertEqual(self.loads(b'cbuiltins\nint\n)R.'), 0)
self.check_unpickling_error(TypeError, b'N)R.')
self.check_unpickling_error(TypeError, b'cbuiltins\nint\nNR.')

def test_bad_newobj(self):
error = (pickle.UnpicklingError, TypeError)
self.assertEqual(self.loads(b'cbuiltins\nint\n)\x81.'), 0)
self.check_unpickling_error(error, b'cbuiltins\nlen\n)\x81.')
self.check_unpickling_error(error, b'cbuiltins\nint\nN\x81.')

def test_bad_newobj_ex(self):
error = (pickle.UnpicklingError, TypeError)
self.assertEqual(self.loads(b'cbuiltins\nint\n)}\x92.'), 0)
self.check_unpickling_error(error, b'cbuiltins\nlen\n)}\x92.')
self.check_unpickling_error(error, b'cbuiltins\nint\nN}\x92.')
self.check_unpickling_error(error, b'cbuiltins\nint\n)N\x92.')

def test_bad_stack(self):
badpickles = [
b'.', # STOP
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Unpickling invalid NEWOBJ_EX opcode with the C implementation raises now
UnpicklingError instead of crashing.
29 changes: 21 additions & 8 deletions Modules/_pickle.c
Original file line number Diff line number Diff line change
Expand Up @@ -5988,23 +5988,30 @@ load_newobj_ex(UnpicklerObject *self)
}

if (!PyType_Check(cls)) {
Py_DECREF(kwargs);
Py_DECREF(args);
PyErr_Format(st->UnpicklingError,
"NEWOBJ_EX class argument must be a type, not %.200s",
Py_TYPE(cls)->tp_name);
Py_DECREF(cls);
return -1;
goto error;
}

if (((PyTypeObject *)cls)->tp_new == NULL) {
Py_DECREF(kwargs);
Py_DECREF(args);
Py_DECREF(cls);
PyErr_SetString(st->UnpicklingError,
"NEWOBJ_EX class argument doesn't have __new__");
return -1;
goto error;
}
if (!PyTuple_Check(args)) {
PyErr_Format(st->UnpicklingError,
"NEWOBJ_EX args argument must be a tuple, not %.200s",
Py_TYPE(args)->tp_name);
goto error;
}
if (!PyDict_Check(kwargs)) {
PyErr_Format(st->UnpicklingError,
"NEWOBJ_EX kwargs argument must be a dict, not %.200s",
Py_TYPE(kwargs)->tp_name);
goto error;
}

obj = ((PyTypeObject *)cls)->tp_new((PyTypeObject *)cls, args, kwargs);
Py_DECREF(kwargs);
Py_DECREF(args);
Expand All @@ -6014,6 +6021,12 @@ load_newobj_ex(UnpicklerObject *self)
}
PDATA_PUSH(self->stack, obj, -1);
return 0;

error:
Py_DECREF(kwargs);
Py_DECREF(args);
Py_DECREF(cls);
return -1;
}

static int
Expand Down

0 comments on commit 4f309ab

Please sign in to comment.