python / python/cpython

Pure Python pickle.py Raises Wrong Exceptions for Invalid Data

オープン
#141,749 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

stdlib type-bug
主要言語
Python
スター
77.2k
フォーク
35.9k
PR マージ指標
PR 指標を取得中

説明

Bug report

Bug description:

Summary

The pure Python implementation of pickle.py in CPython's standard library raises incorrect exceptions (KeyError and IndexError) when unpickling invalid data, instead of the expected pickle.UnpicklingError. This bug is masked in normal CPython usage because the C implementation (_pickle) is used by default, but it affects any Python implementation that relies on the pure Python fallback (e.g., Brython, PyPy in certain modes).

Issue Details

When the pure Python _Unpickler class encounters invalid pickle data, it fails to provide proper error handling in two scenarios:

Problem 1: Invalid Opcodes (KeyError)

When an unrecognized opcode is encountered, dispatch[key[0]](self) raises KeyError instead of UnpicklingError.

Expected behavior: pickle.UnpicklingError: invalid load key, '\xff'.
Actual behavior: KeyError: 255

Problem 2: Missing MARK Instructions (IndexError)

When opcodes like TUPLE are encountered without a preceding MARK instruction, pop_mark() attempts to pop from an empty metastack, raising IndexError instead of UnpicklingError.

Expected behavior: pickle.UnpicklingError: could not find MARK
Actual behavior: IndexError: pop from empty list

Steps to Reproduce

Reproducing in CPython (Force Pure Python Implementation)

The bug exists in CPython's pickle.py but is not normally visible because CPython uses the C _pickle module. To reproduce, you must disable the C implementation:

# test_pickle_bug.py
import sys

# Block the C implementation to force pure Python fallback
sys.modules['_pickle'] = None

import pickle

print("Test 1: Invalid opcode (should raise UnpicklingError)")
try:
    pickle.loads(b'\xff\xff\xff')
    print("FAIL: Expected UnpicklingError")
except pickle.UnpicklingError as e:
    print(f"PASS: {e}")
except KeyError as e:
    print(f"FAIL: Got KeyError instead: {e}")

print("\nTest 2: Missing MARK (should raise UnpicklingError)")
try:
    pickle.loads(b'this is not valid pickle data')
    print("FAIL: Expected UnpicklingError")
except pickle.UnpicklingError as e:
    print(f"PASS: {e}")
except IndexError as e:
    print(f"FAIL: Got IndexError instead: {e}")

Expected output (with C _pickle):

Test 1: Invalid opcode (should raise UnpicklingError)
PASS: invalid load key, '\xff'.

Test 2: Missing MARK (should raise UnpicklingError)
PASS: could not find MARK

Actual output (with pure Python pickle.py):

Test 1: Invalid opcode (should raise UnpicklingError)
FAIL: Got KeyError instead: 255

Test 2: Missing MARK (should raise UnpicklingError)
FAIL: Got IndexError instead: pop from empty list

Root Cause Analysis

Location 1: _Unpickler.load() method

File: Lib/pickle.py
Approximate line: 1212 (may vary by Python version)

try:
    while True:
        key = read(1)
        if not key:
            raise EOFError
        assert isinstance(key, bytes_types)
        dispatch[key[0]](self)  # <-- KeyError raised here for invalid opcodes
except _Stop as stopinst:
    return stopinst.value

The dispatch dictionary lookup is not wrapped in error handling, so KeyError propagates directly to the caller instead of being converted to UnpicklingError.

Location 2: _Unpickler.pop_mark() method

File: Lib/pickle.py
Approximate line: 1219 (may vary by Python version)

def pop_mark(self):
    items = self.stack
    self.stack = self.metastack.pop()  # <-- IndexError raised here if metastack is empty
    self.append = self.stack.append
    return items

When pop_mark() is called without a preceding MARK instruction, the metastack is empty, causing list.pop() to raise IndexError.

Proposed Fix

Add proper error handling to both locations:

Fix 1: Handle KeyError in load() method
try:
    while True:
        key = read(1)
        if not key:
            raise EOFError
        assert isinstance(key, bytes_types)
        try:
            dispatch[key[0]](self)
        except KeyError:
            raise UnpicklingError(
                f"invalid load key, '\\x{key[0]:02x}'.")
except _Stop as stopinst:
    return stopinst.value
Fix 2: Handle empty metastack in pop_mark() method
def pop_mark(self):
    if not self.metastack:
        raise UnpicklingError("could not find MARK")
    items = self.stack
    self.stack = self.metastack.pop()
    self.append = self.stack.append
    return items

Impact

Who is affected?
  • Alternative Python implementations that cannot use the C _pickle module (e.g., Brython, potentially PyPy in pure mode, MicroPython if it includes pickle)
  • Security tools that analyze pickle data and expect proper exception types
  • Testing frameworks that validate pickle error handling
  • Code that explicitly uses pure Python pickle for debugging or compatibility
Why this matters?
  1. API consistency: The pure Python implementation should behave identically to the C implementation
  2. Exception handling: Code that catches pickle.UnpicklingError for malformed data will fail to catch these errors
  3. Security: Malformed pickle data should be handled gracefully with appropriate error messages
  4. Debugging: Proper error messages help developers identify pickle format issues

Verification

After applying the fix, both test cases should pass:

import sys
sys.modules['_pickle'] = None
import pickle

# Test 1: Invalid opcode
try:
    pickle.loads(b'\xff\xff\xff')
    assert False, "Should raise UnpicklingError"
except pickle.UnpicklingError as e:
    assert "invalid load key" in str(e)
    print("✓ Test 1 passed")

# Test 2: Missing MARK
try:
    pickle.loads(b'this is not valid pickle data')
    assert False, "Should raise UnpicklingError"
except pickle.UnpicklingError as e:
    assert "could not find MARK" in str(e)
    print("✓ Test 2 passed")

# Test 3: Using mock file (from unittest.mock)
from io import BytesIO
from unittest.mock import patch, mock_open

invalid_data = b"invalid pickle"
with patch("builtins.open", mock_open(read_data=invalid_data)):
    with open("dummy", "rb") as f:
        try:
            pickle.load(f)
            assert False, "Should raise UnpicklingError"
        except pickle.UnpicklingError:
            print("✓ Test 3 passed")

Additional Context

This issue was discovered while fixing pickle compatibility in Brython. The bug probably exists in all versions of CPython but has gone unnoticed because:

  1. CPython always uses the C _pickle module in practice
  2. The pure Python pickle.py is rarely executed
  3. Most developers don't test with malformed pickle data

However, the pure Python implementation is part of the standard library API contract and should behave identically to the C implementation.

Related Code Patterns

Note that CPython's pickle.py already has precedent for this type of error handling. For example, the load_get(), load_binget(), and load_long_binget() methods all wrap KeyError exceptions when accessing the memo dictionary:

def load_binget(self):
    i = self.read(1)[0]
    try:
        self.append(self.memo[i])
    except KeyError as exc:
        msg = f'Memo value not found at index {i}'
        raise UnpicklingError(msg) from None

The proposed fixes follow this same established pattern.

Python Versions Affected

All Python versions with pure Python pickle.py implementation (verified on 3.9 and 3.14, likely affects 3.9+).

CPython versions tested on:

3.14

Operating systems tested on:

macOS

Linked PRs
  • gh-141751
  • gh-141754

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

Lib/pickle.py から始め、_Unpickler.load() と pop_mark() に注目し、issue で説明されている pure Python fallback を使って 2 つの不正な入力ケースを再現します。無効な opcode と MARK 命令の欠落が、期待されるメッセージを伴う pickle.UnpicklingError を一貫して発生させれば完了です。issue には関連する PR gh-141751 と gh-141754 が記載されているため、作業はすでに進行中かもしれません。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
python
領域
backend
issue の種類
バグ
難易度
3/5
見積もり時間
1〜2日
活発さ
停滞
明瞭さ
明確に書かれている
初心者へのやさしさ
25/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。