diff --git a/Lib/test/pickletester.py b/Lib/test/pickletester.py index 9ba498ce8f575d..83a71b7efedd37 100644 --- a/Lib/test/pickletester.py +++ b/Lib/test/pickletester.py @@ -1246,6 +1246,48 @@ def test_frame_readline(self): # 15: . STOP self.assertEqual(self.loads(pickled), 42) + def test_frame_ends_at_opcode_boundary(self): + # A frame may end exactly between two opcodes; the following opcodes + # are then read from outside the frame. + for pickled in [ + b'\x80\x04\x95\x01\x00\x00\x00\x00\x00\x00\x00N.', # FRAME 1, NONE, STOP + b'\x80\x04\x95\x00\x00\x00\x00\x00\x00\x00\x00N.', # empty FRAME, NONE, STOP + ]: + with self.subTest(pickled=pickled): + self.assertIsNone(self.loads(pickled)) + + def test_frame_does_not_straddle_boundary(self): + # An opcode or its argument must not cross a frame boundary + # (PEP 3154). Such a pickle must be rejected rather than silently + # reading past the declared frame length, which would make the + # meaning of the pickle diverge from its pickletools disassembly. + # See gh-154848. + for pickled in [ + # FRAME 6; UNICODE argument read by readline() straddles the frame. + b'\x80\x04\x95\x06\x00\x00\x00\x00\x00\x00\x00Vhelloworld\n.', + # FRAME 6; BINUNICODE argument straddles the frame. + b'\x80\x04\x95\x06\x00\x00\x00\x00\x00\x00\x00' + b'X\x0a\x00\x00\x00helloworld.', + # FRAME 3; SHORT_BINBYTES argument straddles the frame. + b'\x80\x04\x95\x03\x00\x00\x00\x00\x00\x00\x00C\x0ahelloworld.', + # FRAME 9; GLOBAL argument (second line) straddles the frame. + b'\x80\x04\x95\x09\x00\x00\x00\x00\x00\x00\x00cbuiltins\nprint\n.', + ]: + self.check_unpickling_error(self.truncated_errors, pickled) + + def test_nested_frame(self): + # A new frame must not begin before the current one has ended: here + # the outer frame still has data left after the inner frame header. + pickled = (b'\x80\x04\x95\x0c\x00\x00\x00\x00\x00\x00\x00' + b'N\x95\x00\x00\x00\x00\x00\x00\x00\x00NN.') + self.check_unpickling_error(self.truncated_errors, pickled) + + # But the inner frame header may lie inside the outer frame as long as + # it exactly consumes it. + pickled = (b'\x80\x04\x95\x0a\x00\x00\x00\x00\x00\x00\x00' + b'N\x95\x00\x00\x00\x00\x00\x00\x00\x00N.') + self.assertIsNone(self.loads(pickled)) + def test_compat_unpickle(self): # xrange(1, 7) pickled = b'\x80\x02c__builtin__\nxrange\nK\x01K\x07K\x01\x87R.' diff --git a/Lib/test/test_pickle.py b/Lib/test/test_pickle.py index 48375cf459ea0b..21f45339e8a344 100644 --- a/Lib/test/test_pickle.py +++ b/Lib/test/test_pickle.py @@ -524,7 +524,7 @@ def test_pickler(self): 0) # Write buffer is cleared after every dump(). def test_unpickler(self): - basesize = support.calcobjsize('2P2n3P 2P2n2i5P 2P3n8P2n3i') + basesize = support.calcobjsize('2P2n3P 2P2n2i5P 2P5n8P2n3i') unpickler = _pickle.Unpickler P = struct.calcsize('P') # Size of memo table entry. n = struct.calcsize('n') # Size of mark table entry. diff --git a/Misc/NEWS.d/next/Library/2026-07-29-18-30-00.gh-issue-154848.Fr4meB.rst b/Misc/NEWS.d/next/Library/2026-07-29-18-30-00.gh-issue-154848.Fr4meB.rst new file mode 100644 index 00000000000000..02ca6c6346f55b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-29-18-30-00.gh-issue-154848.Fr4meB.rst @@ -0,0 +1,6 @@ +The :mod:`pickle` C accelerator now enforces frame boundaries when +unpickling, as the pure Python implementation already did. An argument that +straddles a frame boundary, or a frame that begins before the previous one has +ended, now raises :exc:`pickle.UnpicklingError` instead of +being silently read across the boundary. This prevents the loaded data from +diverging from the :mod:`pickletools` disassembly of the same pickle. diff --git a/Modules/_pickle.c b/Modules/_pickle.c index 034dd06b6ab460..90200a4379319d 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -669,6 +669,10 @@ typedef struct UnpicklerObject { Py_ssize_t input_len; Py_ssize_t next_read_idx; Py_ssize_t prefetched_idx; /* index of first prefetched byte */ + Py_ssize_t frame_end; /* End of the current frame, or -1 if not in a + frame. While in a frame, input_len is capped + here so reads can't cross it (PEP 3154). */ + Py_ssize_t saved_input_len; /* input_len to restore when the frame ends. */ PyObject *read; /* read() method of the input stream. */ PyObject *readinto; /* readinto() method of the input stream. */ @@ -1250,6 +1254,7 @@ _Unpickler_SetStringInput(UnpicklerObject *self, PyObject *input) self->input_len = self->buffer.len; self->next_read_idx = 0; self->prefetched_idx = self->input_len; + self->frame_end = -1; return self->input_len; } @@ -1260,6 +1265,30 @@ bad_readline(PickleState *st) return -1; } +/* End the current frame, uncapping input_len. The frame must be fully read. */ +static void +_Unpickler_EndFrame(UnpicklerObject *self) +{ + assert(self->frame_end >= 0); + assert(self->next_read_idx == self->frame_end); + self->input_len = self->saved_input_len; + self->frame_end = -1; +} + +/* Reached the end of the current frame. If straddle, the read crosses the + frame boundary (PEP 3154) and fails; otherwise end the frame. */ +static int +_Unpickler_LeaveFrame(PickleState *st, UnpicklerObject *self, int straddle) +{ + if (straddle) { + PyErr_SetString(st->UnpicklingError, + "pickle exhausted before end of frame"); + return -1; + } + _Unpickler_EndFrame(self); + return 0; +} + /* Skip any consumed data that was only prefetched using peek() */ static int _Unpickler_SkipConsumed(UnpicklerObject *self) @@ -1444,6 +1473,19 @@ _Unpickler_ReadImpl(UnpicklerObject *self, PickleState *st, char **s, Py_ssize_t return -1; } + if (self->frame_end >= 0) { + if (_Unpickler_LeaveFrame(st, self, + self->next_read_idx < self->frame_end) < 0) { + return -1; + } + /* Frame ended; the read may now be satisfied from the buffer. */ + if (n <= self->input_len - self->next_read_idx) { + *s = self->input_buffer + self->next_read_idx; + self->next_read_idx += n; + return n; + } + } + /* This case is handled by the _Unpickler_Read() macro for efficiency */ assert(self->next_read_idx + n > self->input_len); @@ -1489,6 +1531,25 @@ _Unpickler_ReadInto(PickleState *state, UnpicklerObject *self, char *buf, } } + if (self->frame_end >= 0) { + /* in_buffer > 0 is next_read_idx < frame_end on entry: frame data was + consumed above, so the read crosses the frame boundary. */ + if (_Unpickler_LeaveFrame(state, self, in_buffer > 0) < 0) { + return -1; + } + in_buffer = self->input_len - self->next_read_idx; + if (in_buffer > 0) { + Py_ssize_t to_read = Py_MIN(in_buffer, n); + memcpy(buf, self->input_buffer + self->next_read_idx, to_read); + self->next_read_idx += to_read; + buf += to_read; + n -= to_read; + if (n == 0) { + return n; + } + } + } + /* Read from file */ if (!self->read) { /* We're unpickling memory, this means the input is truncated */ @@ -1547,6 +1608,7 @@ _Unpickler_Readline(PickleState *state, UnpicklerObject *self, char **result) { Py_ssize_t i, num_read; +rescan: for (i = self->next_read_idx; i < self->input_len; i++) { if (self->input_buffer[i] == '\n') { char *line_start = self->input_buffer + self->next_read_idx; @@ -1555,6 +1617,14 @@ _Unpickler_Readline(PickleState *state, UnpicklerObject *self, char **result) return _Unpickler_CopyLine(self, line_start, num_read, result); } } + if (self->frame_end >= 0) { + if (_Unpickler_LeaveFrame(state, self, + self->next_read_idx < self->frame_end) < 0) { + return -1; + } + /* Frame ended; continue the line past its end. */ + goto rescan; + } if (!self->read) return bad_readline(state); @@ -1729,6 +1799,8 @@ _Unpickler_New(PyObject *module) self->input_len = 0; self->next_read_idx = 0; self->prefetched_idx = 0; + self->frame_end = -1; + self->saved_input_len = 0; self->read = NULL; self->readinto = NULL; self->readline = NULL; @@ -7078,6 +7150,17 @@ load_frame(PickleState *state, UnpicklerObject *self) if (_Unpickler_Read(self, state, &s, 8) < 0) return -1; + /* A new frame must not begin before the current one ends (PEP 3154). Its + header may lie at the tail of the current frame if it drains it. */ + if (self->frame_end >= 0) { + if (self->next_read_idx < self->frame_end) { + PyErr_SetString(state->UnpicklingError, + "beginning of a new frame before end of current frame"); + return -1; + } + _Unpickler_EndFrame(self); + } + frame_len = calc_binsize(s, 8); if (frame_len < 0) { PyErr_Format(PyExc_OverflowError, @@ -7091,6 +7174,12 @@ load_frame(PickleState *state, UnpicklerObject *self) /* Rewind to start of frame */ self->next_read_idx -= frame_len; + + /* Cap input_len at the frame end so reads can't cross it (PEP 3154); + _Unpickler_EndFrame() restores it when the frame is fully read. */ + self->frame_end = self->next_read_idx + frame_len; + self->saved_input_len = self->input_len; + self->input_len = self->frame_end; return 0; }