Skip to content

execution/rlp, execution/types: reject malformed RLP instead of dropping elements - #21818

Merged
lystopad merged 2 commits into
mainfrom
feature/lystopad/rlp-receipt-eol-fix
Jun 16, 2026
Merged

lystopad merged 2 commits into
mainfrom
feature/lystopad/rlp-receipt-eol-fix

Conversation

@lystopad

Copy link
Copy Markdown
Member

Problem

FuzzRLP found that slice/struct RLP decoding silently accepts malformed input and drops elements instead of erroring. Two independent divergences from go-ethereum:

1. execution/rlp: wrapped EOL swallowed

The generic slice/array/struct element decoders used errors.Is(err, EOL) to detect end-of-list. EOL is a sentinel meaning "the stream reached the end of this list". A custom Decoder (e.g. Receipt.decodePayload) legitimately wraps EOL via fmt.Errorf("...: %w", EOL) on malformed input — errors.Is matched those and treated them as end-of-list, dropping the element and swallowing the error. Switched to the exact sentinel match err == EOL at all three sites, as upstream go-ethereum does.

2. execution/types: empty typed receipt returned the sentinel

Receipt.DecodeRLP returned bare rlp.EOL for an empty typed receipt (0x80). As a slice element that sentinel is read as end-of-list, silently dropping the receipt. Now returns errShortTypedReceipt (the existing "typed receipt too short" error). go-ethereum returns a real error here too; this rlp.EOL traces to a 2019 geth-sync.

Reviewer note: TestDecodeEmptyTypedReceipt previously asserted the rlp.EOL behaviour — it is updated to expect errShortTypedReceipt. This is the one intentional behaviour change.

Impact

Malformed/non-canonical RLP that should be rejected was silently accepted with elements dropped. Bug 1 is type-generic (any []T/struct where T has a custom DecodeRLP that wraps EOL). Main risks: inter-client parsing differential vs go-ethereum (consensus-divergence surface) and silent truncation masking malformed/corrupt data. Not a proven state-forgery — receipts are normally re-checked against the header receiptsRoot — but it closes a malleability/silent-drop class and re-aligns with geth.

Found by / validation

  • Found by FuzzRLP (inputs c2c23030 and c180).
  • execution/rlp + execution/types suites: green.
  • FuzzRLP: 240s / 17M execs, no crash (was crashing in seconds).
  • make lint: 0 issues.

Related (not in this PR)

checkErrListEnd (block.go) uses the same errors.Is(EOL) anti-pattern and is used by the EIP-7928 BAL decoders (which wrap EOL) and block-body decode. There's a partial s.ListEnd() backstop and no fuzzer covering BAL RLP — tracking as a separate follow-up (BAL/block-body RLP fuzzer + checkErrListEnd fix).

Backport

release/3.5 (and likely release/3.4) have both bugs — good cherry-pick candidate.

…ing elements

Two divergences from go-ethereum let slice/struct RLP decoding silently accept
malformed input and drop elements instead of erroring:

1. execution/rlp: the generic slice/array/struct element decoders used
   errors.Is(err, EOL) to detect end-of-list. EOL is a sentinel meaning "the
   stream reached the end of this list". A custom Decoder (e.g.
   Receipt.decodePayload) legitimately wraps EOL via fmt.Errorf("...: %w", EOL)
   on malformed input; errors.Is matched those wrapped EOLs and treated them as
   end-of-list, silently dropping the element and swallowing the error. Use the
   exact sentinel match (err == EOL), as upstream go-ethereum does.

2. execution/types: Receipt.DecodeRLP returned bare rlp.EOL for an empty typed
   receipt (0x80). As a slice element that sentinel is read as end-of-list,
   silently dropping the receipt. Return errShortTypedReceipt, matching the
   "typed receipt too short" handling elsewhere in the file. The existing
   TestDecodeEmptyTypedReceipt is updated: it had enshrined the EOL behaviour.

Found by FuzzRLP (inputs c2c23030 and c180). Validated: execution/rlp and
execution/types suites green; FuzzRLP ran 240s / 17M execs with no crash.
@lystopad
lystopad requested review from mh0lt and yperbasis as code owners June 15, 2026 10:48
@lystopad lystopad self-assigned this Jun 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@lystopad
lystopad added this pull request to the merge queue Jun 16, 2026
Merged via the queue into main with commit 7bc7918 Jun 16, 2026
93 checks passed
@lystopad
lystopad deleted the feature/lystopad/rlp-receipt-eol-fix branch June 16, 2026 07:33
pull Bot pushed a commit to Dustin4444/erigon that referenced this pull request Jun 16, 2026
…fuzz (erigontech#21820)

## What

- **`.github/workflows/test-fuzz.yml`** — scheduled (nightly 03:00 UTC)
active fuzzing of all 23 native Go fuzzers (`testing.F`), one matrix leg
per target, on `ubuntu-latest` (free/unlimited on this public repo),
with a per-target corpus cache. Manually dispatchable with a custom
`fuzztime`.
- **`docs/fuzzing.md`** — targets, local usage, the nightly job, and the
OSS-Fuzz integration
([google/oss-fuzz#15642](google/oss-fuzz#15642)).
- **`make fuzz PKG=<pkg> FUZZ=<FuzzName> [FUZZTIME=60s]`** — local
single-target helper.

## Design

Intentionally **not** part of the CI gate: seed-corpus regression
already runs in `go test ./...`, and putting mutation fuzzing on the
gate would be flaky (a newly found crash would red unrelated PRs). It
complements OSS-Fuzz: runs today before that integration is live, and
also covers the `txnprovider/txpool` (MDBX) targets deferred upstream
(native `go test -fuzz` needs no sanitizer toolchain).

## Notifications (scheduled failures only)

Uploads each crash reproducer as a `fuzz-crash-<target>` artifact,
opens/updates a `nightly-fuzz` tracking issue, and posts to Discord if
the `DISCORD_WEBHOOK` secret is set (no-ops otherwise).

## Notes

- Requires a `DISCORD_WEBHOOK` repository secret for Discord alerts
(optional; safe to merge without).
- Best merged after erigontech#21818 — that fixes the one pre-existing `FuzzRLP`
crash this workflow surfaces, so the first nightly starts green.
yperbasis pushed a commit that referenced this pull request Jun 16, 2026
…f dropping elements (#21819)

Cherry-pick of #21818 to release/3.5.

release/3.5 has both bugs fixed there. Verified: execution/rlp +
execution/types suites green, FuzzRLP clean, make lint 0 issues.
pull Bot pushed a commit to Dustin4444/erigon that referenced this pull request Jun 19, 2026
…ontech#21852)

## What

`checkErrListEnd` (used by the EIP-7928 Block Access List decoder and
block-body decoding) detected end-of-list with `errors.Is(err,
rlp.EOL)`. A nested BAL decoder returns a *wrapped* `EOL` on malformed
input — e.g. an account list with no Address (`AccountChanges.DecodeRLP`
→ `"read Address: %w"` wrapping `rlp.EOL`). `errors.Is` matched the
wrapped EOL and treated it as a clean end-of-list, so the **production**
decoder `DecodeBlockAccessListBytes` **silently truncated** a malformed
BAL to empty instead of erroring: `0xc1c0` (a list with one empty
account) decoded to an empty BAL with no error on `main`.

Fix: match the bare sentinel (`err == rlp.EOL`) so a wrapped EOL
propagates as a real error.

## Severity (corrected from the original description)

This is a **real correctness fix, not no-op hardening** — thanks
@yperbasis for catching this. `DecodeBlockAccessListBytes` is the
production decoder, called from `NewPayload` (`engine_server.go:374`),
where the header commitment is `crypto.HashData` over the **raw** BAL
bytes (`:388`) — so the decoder is the only gate. Once Glamsterdam
activates, silently truncating malformed input would be an accept/reject
divergence vs a strict client. **No mainnet impact today** (BALs are
pre-mainnet, Glamsterdam devnets only). Same spirit as erigontech#21818 ("reject
malformed RLP instead of dropping elements").

The rlp reflection slice decoder already used exact `== EOL` and is
unchanged; block-body callers return a bare EOL at genuine end-of-list,
so the change is scoped to the only vulnerable site.

## Test

`TestBlockAccessListRejectsAddresslessAccount` now goes through
`DecodeBlockAccessListBytes` (the production path) so it actually
exercises `checkErrListEnd` — verified red→green (FAIL on `main` without
the fix, PASS with it). Full `execution/types` suite green; `make lint`
clean.

Co-authored-by: Andrew Ashikhmin <34320705+yperbasis@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants