Commit 5201cf0
authored
chore(audit): round 2 red-team fixes + bindings v0.13.0 (#125)
Round 2 ran five adversarial red-team agents with explicit attacker
mindsets (economic / MEV searcher, operator collusion, governance
takeover, cross-chain bridge, storage / upgrade / init) and Slither
static analysis. ~50 new findings on top of Round 1; this commit lands
the small-but-critical fixes that don't require interface-level
redesign.
CRITICAL — beacon SSZ endianness (B-01)
- `BeaconChainProofs.getEffectiveBalanceGwei`, `getActivationEpoch`,
`getExitEpoch`, `getWithdrawableEpoch`, and `_extractBalanceFromLeaf`
now perform the correct little-endian byte-swap on SSZ-packed uint64
fields. The consensus layer packs values into the HIGH 8 bytes of a
bytes32 chunk in little-endian; the previous code read the LOW 64 bits
as if big-endian, returning 0 (or a byte-swapped wrong value) for
every effective balance, exit epoch, and validator balance proven from
a real EigenPod-CLI fixture. In-tree tests passed only because the
fixtures mirrored the same wrong packing. Fixtures are now SSZ-correct
and `test_extractBalance_RealSszLeaf_32ETH` pins the canonical 32-ETH
encoding.
CRITICAL — TNTLockFactory delegate-on-init airdrop capture (gov #1)
- `getOrCreateLock` requires `msg.sender == beneficiary`. Without the
gate, anyone could front-run the victim's first interaction with the
deterministic lock, supply themselves as `delegatee`, and persistently
capture every future inbound TNT transfer's voting power.
HIGH — JobQuoteDetails missing requester (Round 2 economic F1, BREAKING)
- `Types.JobQuoteDetails` now carries `address requester` and the
`JOB_QUOTE_TYPEHASH` includes it. Any `_permittedCaller` (or anyone
watching the mempool) could previously lift another caller's signed
per-job RFQ quote and consume it for themselves. Off-chain signers
MUST update.
HIGH — wildcard quote rejection
- `verifyQuoteBatch` rejects `requester == address(0)` outright. Wildcard
quotes have no good production use case and let mempool observers race
the legitimate caller for the single-use digest.
HIGH — slash dispute dead-zone (Round 2 economic F4)
- `SlashingLib.disputeSlash` window extends through
`executeAfter + TIMESTAMP_BUFFER`, mirroring `isExecutable`. The prior
asymmetry created a deterministic 15-second window where a sequencer
could land an operator's dispute (revert) and then 15s later anyone
could call `executeSlash`. Operator dispute and execute now use the
same buffer.
HIGH — SLASH_ADMIN self-dispute (Round 2 governance #4)
- A SLASH_ADMIN that is also the proposer can no longer self-dispute.
Without this, a single role-holder could propose, immediately
self-dispute (no bond), and freeze operator stake for the full
`disputeResolutionDeadline` window — and (when treasury == admin)
capture the operator's bond on auto-execution.
HIGH — L2 slash CEI (Round 1 deferred S-1)
- `L2SlashingReceiver._handleSlashMessage` now applies the slash BEFORE
consuming the nonce. If `canSlash` returns false (paused, unknown
operator, etc.) or `slashBps == 0`, the call reverts so the bridge
keeps the message available for retry. Previously the nonce was
consumed first and the slash silently dropped on transient failure,
locking that slash out forever.
HIGH — L2 setMessenger / setSlasher timelock (Round 2 cross-chain C-2)
- Both swaps now require `SENDER_ACTIVATION_DELAY` (2 days) for
non-bootstrap changes. A compromised owner could otherwise hot-swap
the messenger to a contract they control and impersonate any
previously-authorised sender, undercutting the H-4 timelock on
`authorizedSenders`. The first swap (when current is unset) is a
bootstrap exemption so deploy scripts can wire the bridge without
deadlocking.
MEDIUM — MBSM grace-period pinning (Round 1 deferred gov H-3)
- `MBSMRegistry.pinBlueprint` rejects revisions already scheduled for
deprecation. Pinning during the grace window meant `getMBSM` returned
`address(0)` the moment `completeDeprecation` ran, breaking every BSM
call for the pinned blueprint.
MEDIUM — forceRemoveOperator min-operators floor (Round 2 collusion #7)
- A blueprint manager can no longer evict operators below `minOperators`
unless their BSM explicitly opts in via the new
`forceRemoveAllowsBelowMin(serviceId)` hook. The previous unconditional
bypass let a malicious BSM bias the operator set toward sybils.
Default implementation in `BlueprintServiceManagerBase` returns
`false`, enforcing the floor.
MEDIUM — slash `proposeSlash` rejects `bytes32(0)` evidence (Round 1)
- Off-chain monitors keying off non-zero evidence no longer see
silently-zero entries.
LOW — `Types.ServiceRequest.activated` moved to end of struct
- Upgrade-safety: appending the field at the end means a hypothetical
upgrade from a pre-`activated` storage layout cannot accidentally
read a non-zero byte from a different field as `activated == true`.
Test suite
- `test/security/AuditFixesTest.t.sol` adds `disputeSlash` admin self-
dispute regression test on top of the Round 1 entries.
- New `test_extractBalance_RealSszLeaf_32ETH` pins SSZ uint64 decoding.
- `BeaconTestBase._generateValidatorFields` and `_generateBalanceRoot`
now use SSZ-correct packing via the `_sszUint64` / `_reverseUint64`
helpers.
- Updated quote-typehash literals + signing helpers across every test
file that constructs `QuoteDetails` / `JobQuoteDetails`.
- EIP712 cross-repo deterministic vectors regenerated for the new
typehash. Rust `pricing-engine/tests/eip712_compat.rs` MUST be
regenerated to match.
Bindings
- Regenerated via `cargo xtask gen-bindings`. Bumped to 0.13.0
(BREAKING: typehash changes; consumers MUST update their EIP-712
payloads).
- Slither sweep: 1279 results, mostly unused-state-variable +
cache-array-length + constable-states (all informational on
upgradeable contracts). Two real findings flagged for follow-up:
`ServiceFeeDistributor._claimAllForToken` reentrancy-eth pattern
(state-after-external-call inside a nonReentrant-guarded entry,
needs CEI review) and `RebasingAssetAdapter` first-depositor
inflation surface (needs virtual-offset migration; deferred).
Findings deferred for follow-up PRs (out of scope here)
- Cross-chain C-3: receiver-rotation replay across re-deployments.
- Cross-chain H-1: Arbitrum L2 fee leak to inaccessible alias.
- Cross-chain H-5: BSM `onSlash` bypass on beacon-slash path.
- Cross-chain H-6: multi-pod operator under-slash by ~9-15%.
- Operator collusion 2c: bitmap snapshot binding for aggregated jobs
(swap-and-pop reorder breaks in-flight signatures).
- Economic F2: `RebasingAssetAdapter` virtual-offset migration.
- Economic F3: `cancelSlash` bond-refund reentrancy via disputer.
- Economic F5: stake-just-in-time `billSubscription` arbitrage.
- Economic F6: fee-on-transfer / rebasing token escrow drift.
- Storage F-3: missing `__gap` on five rewards/* UUPS implementations.
- Governance #5: ERC20Burnable burn-before-snapshot quorum suppression.
- Governance #8: 50-action proposals + low threshold = privileged-call
obfuscation surface.
- `ServiceFeeDistributor._claimAllForToken` reentrancy review.
The two governance upgrade tests pre-existing on origin/main
(`test_GovernorSelfUpgrade`, `test_TokenUpgradeViaGovernance`) remain
broken; not introduced by this PR.1 parent 23bb5f6 commit 5201cf0
44 files changed
Lines changed: 1066 additions & 358 deletions
File tree
- bindings
- abi
- src/bindings
- fixtures
- script
- src
- beacon
- core
- governance/lockups
- interfaces
- libraries
- test
- beacon
- security
- tangle
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
10 | 97 | | |
11 | 98 | | |
12 | 99 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
0 commit comments