Skip to content

cluster: reject fixed size fields of the wrong length - #4806

Open
OisinKyne wants to merge 1 commit into
mainfrom
oisin/strict-lock-verification
Open

OisinKyne wants to merge 1 commit into
mainfrom
oisin/strict-lock-verification

Conversation

@OisinKyne

@OisinKyne OisinKyne commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

putBytesN used to zero-pad short values, so, for example, 20-byte withdrawal credentials hashed the same as their padded 32-byte form and passed lock verification. Fixed-size fields in definitions and locks must now be empty or exactly the right length. This is checked in VerifyHashes and when hashing, not when unmarshalling, so --no-verify still loads such a lock. The lock aggregate and node signature lengths are checked in VerifySignatures.

Adds --no-verify to exit sign, exit fetch and exit broadcast, so an operator whose lock fails verification can still exit. Exit messages are still verified by the Obol API and the beacon chain. The golden lock testdata is updated to use real-length signatures. The hashes are unchanged.

The deposit data, registration and address checks and the spec test vectors that used to be in this PR have moved to follow-ups.

category: bug
ticket: none

@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.31343% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.81%. Comparing base (e7d8587) to head (35bce88).

Files with missing lines Patch % Lines
cluster/ssz.go 69.76% 13 Missing ⚠️
cluster/definition.go 92.85% 2 Missing ⚠️
cmd/exit.go 83.33% 1 Missing ⚠️
eth2util/deposit/deposit.go 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4806      +/-   ##
==========================================
+ Coverage   67.72%   67.81%   +0.08%     
==========================================
  Files         255      255              
  Lines       33711    33808      +97     
==========================================
+ Hits        22832    22928      +96     
- Misses      10878    10879       +1     
  Partials        1        1              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

putBytesN zero-padded short values, so for example 20 byte withdrawal credentials
hashed the same as their padded 32 byte form and passed lock verification. Fixed
size definition and lock fields must now be empty or exactly the right length,
checked by VerifyHashes and when hashing. Lock aggregate and node signature lengths
are checked by VerifySignatures. Unmarshalling stays lenient so --no-verify can
still load such a lock.

Adds --no-verify to exit sign, fetch and broadcast so operators can still exit with
a lock that fails verification; exit messages are still verified by the Obol API
and the beacon chain.

category: bug
ticket: none
@OisinKyne
OisinKyne force-pushed the oisin/strict-lock-verification branch from a1e76b1 to 35bce88 Compare November 4, 2026 01:32
@sonarqubecloud

sonarqubecloud Bot commented Nov 4, 2026

Copy link
Copy Markdown

@OisinKyne OisinKyne changed the title cluster: reject malformed fields, verify lock deposit data cluster: reject fixed size fields of the wrong length Nov 4, 2026
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.

2 participants