summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Lib/test/pickletester.py85
-rw-r--r--Lib/test/test_pickle.py3
-rw-r--r--Misc/NEWS2
-rw-r--r--Modules/_pickle.c19
4 files changed, 105 insertions, 4 deletions
diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py
index e852b01..cd2423f 100644
--- a/Lib/test/pickletester.py
+++ b/Lib/test/pickletester.py
@@ -12,7 +12,7 @@ import weakref
from http.cookies import SimpleCookie
from test.support import (
- TestFailed, TESTFN, run_with_locale, no_tracing,
+ TestFailed, TESTFN, run_with_locale, no_tracing, captured_stdout,
_2G, _4G, bigmemtest,
)
@@ -987,6 +987,89 @@ class AbstractUnpickleTests(unittest.TestCase):
self.assertIs(type(unpickled), collections.UserDict)
self.assertEqual(unpickled, collections.UserDict({1: 2}))
+ def test_bad_stack(self):
+ badpickles = [
+ b'0.', # POP
+ b'1.', # POP_MARK
+ b'2.', # DUP
+ # b'(2.', # PyUnpickler doesn't raise
+ b'R.', # REDUCE
+ b')R.',
+ b'a.', # APPEND
+ b'Na.',
+ b'b.', # BUILD
+ b'Nb.',
+ b'd.', # DICT
+ b'e.', # APPENDS
+ # b'(e.', # PyUnpickler raises AttributeError
+ b'ibuiltins\nlist\n.', # INST
+ b'l.', # LIST
+ b'o.', # OBJ
+ b'(o.',
+ b'p1\n.', # PUT
+ b'q\x00.', # BINPUT
+ b'r\x00\x00\x00\x00.', # LONG_BINPUT
+ b's.', # SETITEM
+ b'Ns.',
+ b'NNs.',
+ b't.', # TUPLE
+ b'u.', # SETITEMS
+ b'(u.',
+ b'}(Nu.',
+ b'\x81.', # NEWOBJ
+ b')\x81.',
+ b'\x85.', # TUPLE1
+ b'\x86.', # TUPLE2
+ b'N\x86.',
+ b'\x87.', # TUPLE3
+ b'N\x87.',
+ b'NN\x87.',
+ b'\x90.', # ADDITEMS
+ # b'(\x90.', # PyUnpickler raises AttributeError
+ b'\x91.', # FROZENSET
+ b'\x92.', # NEWOBJ_EX
+ b')}\x92.',
+ b'\x93.', # STACK_GLOBAL
+ b'Vlist\n\x93.',
+ b'\x94.', # MEMOIZE
+ ]
+ for p in badpickles:
+ with self.subTest(p):
+ self.assertRaises(self.bad_stack_errors, self.loads, p)
+
+ def test_bad_mark(self):
+ badpickles = [
+ b'cbuiltins\nlist\n)(R.', # REDUCE
+ b'cbuiltins\nlist\n()R.',
+ b']N(a.', # APPEND
+ b'cbuiltins\nValueError\n)R}(b.', # BUILD
+ b'cbuiltins\nValueError\n)R(}b.',
+ b'(Nd.', # DICT
+ b'}NN(s.', # SETITEM
+ b'}N(Ns.',
+ b'cbuiltins\nlist\n)(\x81.', # NEWOBJ
+ b'cbuiltins\nlist\n()\x81.',
+ b'N(\x85.', # TUPLE1
+ b'NN(\x86.', # TUPLE2
+ b'N(N\x86.',
+ b'NNN(\x87.', # TUPLE3
+ b'NN(N\x87.',
+ b'N(NN\x87.',
+ b'cbuiltins\nlist\n)}(\x92.', # NEWOBJ_EX
+ b'cbuiltins\nlist\n)(}\x92.',
+ b'cbuiltins\nlist\n()}\x92.',
+ b'Vbuiltins\n(Vlist\n\x93.', # STACK_GLOBAL
+ b'Vbuiltins\nVlist\n(\x93.',
+ ]
+ for p in badpickles:
+ # PyUnpickler prints reduce errors to stdout
+ with self.subTest(p), captured_stdout():
+ try:
+ self.loads(p)
+ except (IndexError, AttributeError, TypeError,
+ pickle.UnpicklingError):
+ pass
+
class AbstractPickleTests(unittest.TestCase):
# Subclass must define self.dumps, self.loads.
diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py
index 8dc93d2..f04c5ac 100644
--- a/Lib/test/test_pickle.py
+++ b/Lib/test/test_pickle.py
@@ -32,6 +32,7 @@ class PickleTests(AbstractPickleModuleTests):
class PyUnpicklerTests(AbstractUnpickleTests):
unpickler = pickle._Unpickler
+ bad_stack_errors = (IndexError,)
def loads(self, buf, **kwds):
f = io.BytesIO(buf)
@@ -62,6 +63,7 @@ class InMemoryPickleTests(AbstractPickleTests, AbstractUnpickleTests,
pickler = pickle._Pickler
unpickler = pickle._Unpickler
+ bad_stack_errors = (pickle.UnpicklingError, IndexError)
def dumps(self, arg, protocol=None):
return pickle.dumps(arg, protocol)
@@ -119,6 +121,7 @@ class PyChainDispatchTableTests(AbstractDispatchTableTests):
if has_c_implementation:
class CUnpicklerTests(PyUnpicklerTests):
unpickler = _pickle.Unpickler
+ bad_stack_errors = (pickle.UnpicklingError,)
class CPicklerTests(PyPicklerTests):
pickler = _pickle.Pickler
diff --git a/Misc/NEWS b/Misc/NEWS
index 635da46..dee8338 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -106,6 +106,8 @@ Core and Builtins
Library
-------
+- Issue #23914: Fixed SystemError raised by unpickler on broken pickle data.
+
- Issue #25691: Fixed crash on deleting ElementTree.Element attributes.
- Issue #25624: ZipFile now always writes a ZIP_STORED header for directory
diff --git a/Modules/_pickle.c b/Modules/_pickle.c
index 8a98f51..d3bc420 100644
--- a/Modules/_pickle.c
+++ b/Modules/_pickle.c
@@ -448,8 +448,8 @@ Pdata_grow(Pdata *self)
static PyObject *
Pdata_pop(Pdata *self)
{
- PickleState *st = _Pickle_GetGlobalState();
if (Py_SIZE(self) == 0) {
+ PickleState *st = _Pickle_GetGlobalState();
PyErr_SetString(st->UnpicklingError, "bad pickle data");
return NULL;
}
@@ -5079,6 +5079,9 @@ load_obj(UnpicklerObject *self)
if ((i = marker(self)) < 0)
return -1;
+ if (Py_SIZE(self->stack) - i < 1)
+ return stack_underflow();
+
args = Pdata_poptuple(self->stack, i + 1);
if (args == NULL)
return -1;
@@ -5737,13 +5740,18 @@ do_append(UnpicklerObject *self, Py_ssize_t x)
static int
load_append(UnpicklerObject *self)
{
+ if (Py_SIZE(self->stack) - 1 <= 0)
+ return stack_underflow();
return do_append(self, Py_SIZE(self->stack) - 1);
}
static int
load_appends(UnpicklerObject *self)
{
- return do_append(self, marker(self));
+ Py_ssize_t i = marker(self);
+ if (i < 0)
+ return -1;
+ return do_append(self, i);
}
static int
@@ -5793,7 +5801,10 @@ load_setitem(UnpicklerObject *self)
static int
load_setitems(UnpicklerObject *self)
{
- return do_setitems(self, marker(self));
+ Py_ssize_t i = marker(self);
+ if (i < 0)
+ return -1;
+ return do_setitems(self, i);
}
static int
@@ -5803,6 +5814,8 @@ load_additems(UnpicklerObject *self)
Py_ssize_t mark, len, i;
mark = marker(self);
+ if (mark < 0)
+ return -1;
len = Py_SIZE(self->stack);
if (mark > len || mark <= 0)
return stack_underflow();