Commit 68740e6
authored
fix: spec-faithful aggregation_bits bounds handling in process_attestations (#425)
## Problem (audit finding C1)
`process_attestations` incremented `attestations_processed` and inserted
an empty votes entry into `justifications` *before* the oversized
`aggregation_bits` length check. A skipped attestation therefore still
added an all-false justification root that got serialized into the
post-state: a cross-client state-divergence vector.
## Changes
**Commit 1** moves the bounds check above the `entry()` insert, so a
skipped attestation leaves no trace in the post-state.
**Commit 2** resolves the skip-vs-reject question by checking leanSpec
at the pinned commit (`f12000b`,
`src/lean_spec/forks/lstar/spec.py::process_attestations`). The spec has
**no bitlist length check at all**. Its actual failure modes:
| Case | Spec behavior | ethlambda before | ethlambda after |
|------|--------------|------------------|-----------------|
| Set bit at index >= validator count | `IndexError` -> STF aborts ->
**block rejected** | attestation skipped, block accepted | block
rejected (`Error::AggregationBitsOutOfBounds`) |
| No bits set | `AssertionError` in `to_validator_indices` -> **block
rejected** | processed as no-op + spurious all-false entry in post-state
| block rejected (`Error::EmptyAggregationBits`) |
| Oversized bitlist, all set bits in range | processed normally |
attestation skipped (post-state divergence) | processed normally |
Zeam and Lantern also reject such blocks (per the pre-existing in-code
comment), so the previous skip-while-accepting behavior could split
ethlambda off from the rest of the network on a crafted block.
The checks sit after `is_valid_vote`, matching the spec's filter order:
an attestation that fails the vote-validity filters is skipped *without*
touching its bits, exactly as the spec `continue`s before any bits
access.
## Testing
- `cargo clippy --workspace --all-targets -- -D warnings` clean.
- STF spectests currently fail to *deserialize* fixtures locally
(`missing field attestationPubkey`) because the local `leanSpec`
checkout drifted off the pin; pre-existing and unrelated. CI regenerates
fixtures from the pin.1 parent 47b7889 commit 68740e6
1 file changed
Lines changed: 28 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
40 | 47 | | |
41 | 48 | | |
42 | 49 | | |
| |||
276 | 283 | | |
277 | 284 | | |
278 | 285 | | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
279 | 306 | | |
280 | 307 | | |
281 | 308 | | |
282 | 309 | | |
283 | 310 | | |
284 | | - | |
285 | | - | |
286 | | - | |
287 | | - | |
288 | | - | |
289 | | - | |
290 | | - | |
291 | | - | |
292 | | - | |
| 311 | + | |
293 | 312 | | |
294 | 313 | | |
295 | 314 | | |
| |||
0 commit comments