trailofbits / trailofbits/fickling
Document that an unimplemented opcode raises rather than being skipped (and 4 missing opcodes)
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 670
- Forks
- 77
- PR merge metrics
- No merged PRs in 30d
Description
Summary
Fickling's opcode table is missing four opcodes that CPython's pickletools knows, and the safety of that gap rests entirely on an undocumented behaviour: Opcode.__new__ raises on an unimplemented opcode rather than skipping it. That failure mode is load-bearing for downstream tools, and I would like to see it stated as a guarantee.
This is not a vulnerability report — the current behaviour is safe. It is a request to document what makes it safe, plus four opcodes you may want to add.
The gap
>>> import pickletools
>>> from fickling.fickle import OPCODES_BY_NAME
>>> sorted({op.name for op in pickletools.opcodes} - set(OPCODES_BY_NAME))
['BYTEARRAY8', 'FLOAT', 'NEXT_BUFFER', 'READONLY_BUFFER']
68 opcodes in CPython, 64 in fickling. FLOAT is protocol 0, not an exotic protocol-5 addition.
Why it matters, and why it is currently fine
A pickle can be made whose prefix fickling cannot decode but CPython walks straight through:
>>> import io, pickletools
>>> data = b"}" + b"F1.0\n" + b"cos\nsystem\n(S'echo pwned'\ntR."
>>> [op.name for op, _, _ in pickletools.genops(io.BytesIO(data))]
['EMPTY_DICT', 'FLOAT', 'GLOBAL', 'MARK', 'STRING', 'TUPLE', 'REDUCE', 'STOP']
CPython reaches GLOBAL and REDUCE. Fickling cannot get past FLOAT.
That is not exploitable today, because the dispatch in Opcode.__new__ raises instead of continuing:
raise NotImplementedError(f"TODO: Add support for Opcode {info.name}")
The file therefore surfaces as an error, and a caller that treats errors as unsafe is protected. The important detail is that this is a single central dispatch, so it holds uniformly for all four and for any future unknown opcode — the safe behaviour is a property of the architecture, not an accident of these particular opcodes.
The request
Document that an unimplemented opcode raises rather than being skipped, as a guarantee rather than an implementation detail.
Downstream tools depend on it whether they realise it or not. Ours does: we treat "fickling could not parse this" as "do not report this file as clean". If the dispatch were ever relaxed to skip unknown opcodes — a plausible, well-intentioned robustness change — every such tool would silently start reporting these files as safe, and nothing in the current documentation says that would be a breaking change.
Adding the four opcodes is worth doing and is additive. But it narrows this particular gap rather than closing the class: the table can drift from CPython again at the next protocol addition, and the loud failure is what makes that drift safe.
A note on your reporting guidelines
SECURITY.md asks for reproductions via the opcode API rather than raw byte strings, and I have followed that elsewhere. It is not possible here by construction: the finding is precisely that FLOAT, BYTEARRAY8, NEXT_BUFFER and READONLY_BUFFER have no representation in the opcode API, because they are not implemented. Raw bytes are the only way to express a pickle containing them — which is itself a small argument for adding them.
I have used echo in the proof of concept, per the same guidance.
Versions
master at 854c99a; also reproduced on the 0.1.12 release.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at Opcode.new and the OPCODES_BY_NAME table in fickling.fickle, then compare the table with Python's pickletools.opcodes. Document that unsupported opcodes raise NotImplementedError rather than being skipped, and assess the four named missing opcodes for addition. Done means the safety guarantee is documented and any opcode changes are covered by the project's existing checks.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- security
- Issue type
- Documentation
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 64/100