Skip to content

Fix nil deref, swallowed cache read errors and log truncation cap#821

Draft
sduchesneau wants to merge 3 commits into
developfrom
fix/engine-cache-wasm-bugs
Draft

Fix nil deref, swallowed cache read errors and log truncation cap#821
sduchesneau wants to merge 3 commits into
developfrom
fix/engine-cache-wasm-bugs

Conversation

@sduchesneau

Copy link
Copy Markdown
Contributor

Fixes three confirmed bugs in the engine/cache/wasm paths.

Fixes

  1. pipeline/exec/sharedcache.goSharedCache.Execute called inst.Close(ctx) before checking the error from ExecuteNewCall. The wazero runtime returns (nil, err) on instantiation or heap-write failure, so the nil interface dereference panicked in the tier1 shared-cache path and was recovered upstream as a generic "unknown error", masking the real cause. Now only closes a non-nil instance so the real error propagates.

  2. pipeline/cache/engine.goEngine.NewBuffer checked if !ok { continue } before if err != nil. fileReader.Get returns (nil, false, err) on read errors, so corrupt/failed execout cache reads were silently treated as "block absent", producing silent data gaps instead of a failure. The error check now comes first.

  3. wasm/call.goReachedLogsMaxByteCount compared the cumulative LogsByteCount against maxLogByteCount (512 KiB per-message cap) instead of maxTotalLogsByteCount (5 MiB total cap), truncating module logs 10x earlier than the truncation message itself claims. Now compares against the total cap.

Tests

  • pipeline/exec/sharedcache_test.go — fake wasm.Module whose ExecuteNewCall returns (nil, err): Execute must return the error without panicking.
  • pipeline/cache/engine_test.go — fake execout.FileReader: a read error must fail NewBuffer; absent blocks are still skipped; found payloads are still loaded into the buffer.
  • wasm/call_logs_test.go — cumulative logs above 512 KiB but below 5 MiB must not be truncated; logs reaching 5 MiB must be truncated with the marker message.

All new tests fail on the unfixed code and pass after the fixes. Full go test ./pipeline/... ./wasm/... passes.

🤖 Generated with Claude Code

ExecuteNewCall returns a nil instance on instantiation failure;
closing it unconditionally panicked and masked the real error.
Get errors were checked after the found flag, so corrupt cache
reads were silently treated as absent blocks.
ReachedLogsMaxByteCount compared against the 512 KiB per-message
cap, truncating logs 10x earlier than documented.
@sduchesneau

Copy link
Copy Markdown
Contributor Author

🔍 Vulnerabilities of ghcr.io/streamingfast/substreams:eac5b6a

📦 Image Reference ghcr.io/streamingfast/substreams:eac5b6a
digestsha256:9dfeb76afbaa988883cb07152a627ecdc760f26d90782d312d92b643838d09e6
vulnerabilitiescritical: 0 high: 0 medium: 0 low: 0
platformlinux/amd64
size108 MB
packages353
📦 Base Image ubuntu:24.04
also known as
  • noble
  • noble-20260610
digestsha256:52df9b1ee71626e0088f7d400d5c6b5f7bb916f8f0c82b474289a4ece6cf3faf
vulnerabilitiescritical: 0 high: 0 medium: 12 low: 5

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.

1 participant