diff --git a/core/blockchain.go b/core/blockchain.go index f6873cdb1a..0542fe6be7 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -3678,7 +3678,7 @@ func (bc *BlockChain) insertSideChain(block *types.Block, it *insertIterator, ma continue } - if canonical != nil && canonical.Root() == block.Root() { + if canonical != nil && canonical.Root() == block.Root() && bc.isSidechainGhostState(block, canonical) { // This is most likely a shadow-state attack. When a fork is imported into the // database, and it eventually reaches a block height which is not pruned, we // just found that the state already exist! This means that the sidechain block @@ -3797,6 +3797,34 @@ func (bc *BlockChain) insertSideChain(block *types.Block, it *insertIterator, ma return nil, 0, nil } +// isSidechainGhostState decides whether a sidechain block whose state root +// already matches the canonical block at the same height is a shadow-state +// attack (true) or a legitimate no-op block (false). It is only meaningful when +// canonical.Root() == block.Root() already holds. +// +// A block that performs no state transition (e.g. an empty block) inherits its +// parent's state root, so two distinct no-op blocks at the same height (differing +// only in seal/timestamp/coinbase) legitimately share a state root: there is no +// forged state to skip-verify. We exempt that case, but only when the *canonical* +// chain was also a no-op at this height, measured against the canonical parent's +// trusted (already-verified) header. Gating on the canonical side, not just the +// side block's own parent, closes the forge-both-roots bypass: the side block's +// parent may itself be unverified/attacker-controlled here, whereas the canonical +// parent header cannot be. If either parent header is unavailable, we keep the +// conservative behavior and treat it as an attack. +func (bc *BlockChain) isSidechainGhostState(block *types.Block, canonical *types.Block) bool { + canonParent := bc.GetHeaderByNumber(canonical.NumberU64() - 1) + sideParent := bc.GetHeader(block.ParentHash(), block.NumberU64()-1) + if canonParent == nil || sideParent == nil { + return true + } + // Legitimate only if neither the canonical block nor the side block changed + // state relative to its own parent, i.e. both are genuine no-ops. + canonNoOp := canonParent.Root == canonical.Root() + sideNoOp := sideParent.Root == block.Root() + return !(canonNoOp && sideNoOp) +} + // recoverAncestors finds the closest ancestor with available state and re-execute // all the ancestor blocks since that. // recoverAncestors is only used post-merge. diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 0a2cd47a9f..30cc4c60ad 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -3108,6 +3108,240 @@ func testSideImportPrunedBlocks(t *testing.T, scheme string) { } } +// TestSideImportEmptyBlockGhostState is a regression test for +// https://github.com/0xPolygon/bor/issues/2224. Two distinct empty +// (zero-transaction) blocks at the same height legitimately share a state root, +// because neither performs a state transition: both inherit their parent's root. +// insertSideChain used to reject any sidechain block whose root matched the +// canonical block at the same height as a "ghost-state attack", which fired a +// false positive on such empty blocks and wedged sync after a validator restart. +func TestSideImportEmptyBlockGhostState(t *testing.T) { + testSideImportEmptyBlockGhostState(t, rawdb.HashScheme) + testSideImportEmptyBlockGhostState(t, rawdb.PathScheme) +} + +// noRewardEngine wraps the faker but skips the block reward, so empty blocks +// make no state transition and share their parent's state root — matching how +// bor's reward-less empty blocks behave (the condition that triggers #2224). +type noRewardEngine struct{ *ethash.Ethash } + +func (e *noRewardEngine) Finalize(chain consensus.ChainHeaderReader, header *types.Header, state vm.StateDB, body *types.Body, receipts []*types.Receipt) ([]*types.Receipt, error) { + return receipts, nil +} + +func (e *noRewardEngine) FinalizeAndAssemble(chain consensus.ChainHeaderReader, header *types.Header, state *state.StateDB, body *types.Body, receipts []*types.Receipt) (*types.Block, []*types.Receipt, time.Duration, error) { + header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number)) + return types.NewBlock(header, &types.Body{Transactions: body.Transactions, Uncles: body.Uncles, Withdrawals: body.Withdrawals}, receipts, trie.NewStackTrie(nil)), receipts, 0, nil +} + +// ghostStateTestKey funds the sender used to drive state transitions in the +// ghost-state sidechain tests. +var ghostStateTestKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + +// setupPrunedGhostStateChain builds a canonical chain whose blocks all perform a +// state transition (so old states get pruned), inserts it, and returns the chain +// plus everything a sidechain test needs to fork at forkIdx. forkBlockEmpty +// controls whether the canonical block at forkIdx is left empty (no state +// transition) or carries the same value-transfer tx as every other block. It +// asserts the fork parent's state is pruned, so re-importing a sibling routes +// through insertSideChain via ErrPrunedAncestor. +func setupPrunedGhostStateChain(t *testing.T, scheme string, forkBlockEmpty bool) (chain *BlockChain, genDb ethdb.Database, blocks []*types.Block, forkParent *types.Block, forkIdx int, forkTxNonce uint64) { + t.Helper() + + engine := &noRewardEngine{ethash.NewFaker()} + addr := crypto.PubkeyToAddress(ghostStateTestKey.PublicKey) + signer := types.LatestSigner(params.TestChainConfig) + genesis := &Genesis{ + Config: params.TestChainConfig, + Alloc: types.GenesisAlloc{addr: {Balance: big.NewInt(1e18)}}, + BaseFee: big.NewInt(params.InitialBaseFee), + } + + states := state.TriesInMemory + if scheme == rawdb.PathScheme { + states = state.TriesInMemory + 1 + } + total := 2 * state.TriesInMemory + // forkIdx indexes into blocks (blocks[i] is height i+1) and sits comfortably + // below the pruning boundary. + forkIdx = total - states - 5 + + nonce := uint64(0) + genDb, blocks, _ = GenerateChainWithGenesis(genesis, engine, total, func(i int, b *BlockGen) { + b.SetCoinbase(common.Address{1}) + if i == forkIdx { + forkTxNonce = nonce + if forkBlockEmpty { + return // no state transition: root stays equal to parent's + } + } + tx, _ := types.SignTx(types.NewTransaction(nonce, common.Address{0xaa}, big.NewInt(1), params.TxGas, b.BaseFee(), nil), signer, ghostStateTestKey) + b.AddTx(tx) + nonce++ + }) + + datadir := t.TempDir() + pdb, err := pebble.New(datadir, 0, 0, "", false) + if err != nil { + t.Fatalf("Failed to create persistent key-value database: %v", err) + } + db, err := rawdb.Open(pdb, rawdb.OpenOptions{Ancient: path.Join(datadir, "ancient")}) + if err != nil { + t.Fatalf("Failed to create persistent freezer database: %v", err) + } + t.Cleanup(func() { db.Close() }) + + chain, err = NewBlockChain(db, genesis, engine, DefaultConfig().WithStateScheme(scheme)) + if err != nil { + t.Fatalf("failed to create tester chain: %v", err) + } + t.Cleanup(chain.Stop) + + if n, err := chain.InsertChain(blocks, false); err != nil { + t.Fatalf("block %d: failed to insert into chain: %v", n, err) + } + + forkParent = blocks[forkIdx-1] + if chain.HasState(forkParent.Root()) { + t.Fatalf("test setup: fork parent (block %d) state not pruned", forkParent.NumberU64()) + } + return chain, genDb, blocks, forkParent, forkIdx, forkTxNonce +} + +func testSideImportEmptyBlockGhostState(t *testing.T, scheme string) { + chain, genDb, blocks, forkParent, forkIdx, _ := setupPrunedGhostStateChain(t, scheme, true) + + // Empty sibling at the fork height: different coinbase → different hash, but + // same (parent's) state root as both its parent and the canonical block. + sideBlocks, _ := GenerateChain(params.TestChainConfig, forkParent, chain.engine, genDb, 1, func(i int, b *BlockGen) { + b.SetCoinbase(common.Address{2}) + }) + side := sideBlocks[0] + canonical := blocks[forkIdx] + if side.Hash() == canonical.Hash() { + t.Fatalf("side block hash must differ from canonical") + } + if side.Root() != canonical.Root() { + t.Fatalf("test setup: side root %x must equal canonical root %x", side.Root(), canonical.Root()) + } + if side.Root() != forkParent.Root() { + t.Fatalf("test setup: empty side block must inherit parent root") + } + + if _, err := chain.InsertChain(sideBlocks, false); err != nil { + t.Fatalf("empty side block falsely rejected: %v", err) + } +} + +// TestSideImportGhostStateStillRejected is the security counterpart to +// TestSideImportEmptyBlockGhostState: it proves the #2224 fix only narrowed the +// false positive and did not open the real hole. A side block that DID perform a +// state transition (its root differs from its parent's) but claims a state root +// equal to the canonical block at that height is the genuine shadow-state attack +// and must still be rejected. +func TestSideImportGhostStateStillRejected(t *testing.T) { + testSideImportGhostStateStillRejected(t, rawdb.HashScheme) + testSideImportGhostStateStillRejected(t, rawdb.PathScheme) +} + +func testSideImportGhostStateStillRejected(t *testing.T, scheme string) { + chain, genDb, blocks, forkParent, forkIdx, forkTxNonce := setupPrunedGhostStateChain(t, scheme, false) + signer := types.LatestSigner(params.TestChainConfig) + + // Side block on the canonical fork parent, carrying the SAME tx as the + // canonical fork block (with a different coinbase). It reaches the canonical + // root via a real state transition, so its root differs from its parent's: + // side.Root() == canonical.Root() but side.Root() != forkParent.Root(). + sideBlocks, _ := GenerateChain(params.TestChainConfig, forkParent, chain.engine, genDb, 1, func(i int, b *BlockGen) { + b.SetCoinbase(common.Address{2}) + tx, _ := types.SignTx(types.NewTransaction(forkTxNonce, common.Address{0xaa}, big.NewInt(1), params.TxGas, b.BaseFee(), nil), signer, ghostStateTestKey) + b.AddTx(tx) + }) + side := sideBlocks[0] + canonical := blocks[forkIdx] + if side.Root() != canonical.Root() { + t.Fatalf("test setup: side root %x must equal canonical root %x", side.Root(), canonical.Root()) + } + if side.Root() == forkParent.Root() { + t.Fatalf("test setup: side block must change state (root must differ from parent)") + } + + if _, err := chain.InsertChain(sideBlocks, false); err == nil || err.Error() != "sidechain ghost-state attack" { + t.Fatalf("expected ghost-state attack rejection, got: %v", err) + } +} + +// TestSidechainGhostStateCanonicalGate covers the hardening requested in PR #2293 +// review: the no-op exemption must be gated on the *canonical* chain also being a +// no-op at that height, not only on the side block's own (possibly attacker-forged) +// parent. This defends against an attacker who forges both side parent and side +// child roots to match a canonical root at a height where the canonical chain +// actually changed state (the forge-both-roots bypass). Because the side block's +// parent can be unverified here while the canonical parent header is trusted, +// isSidechainGhostState decides on the canonical parent. This exercises the +// decision directly, since forging a header past GenerateChain's honest state +// transitions is not otherwise reachable. +func TestSidechainGhostStateCanonicalGate(t *testing.T) { + chain, _, blocks, _, forkIdx, _ := setupPrunedGhostStateChain(t, rawdb.HashScheme, false) + + // canonical is a real state-transition block: its root differs from its parent. + canonical := blocks[forkIdx] + canonParent := blocks[forkIdx-1] + if canonical.Root() == canonParent.Root() { + t.Fatalf("test setup: canonical block must change state relative to its parent") + } + + // Forge a side parent that already sits at the canonical root, then a no-op + // side child inheriting it: sideNoOp is true. But the canonical chain changed + // state here (canonNoOp false), so the exemption must be denied. The forged + // side parent is written to the header chain so GetHeader resolves it, mirroring + // how insertSideChain writes sidechain headers without state verification. + forgedParent := types.NewBlockWithHeader(&types.Header{ + Number: canonParent.Number(), + ParentHash: canonParent.ParentHash(), + Root: canonical.Root(), + Coinbase: common.Address{9}, + }) + rawdb.WriteHeader(chain.db, forgedParent.Header()) + forged := types.NewBlockWithHeader(&types.Header{ + Number: canonical.Number(), + ParentHash: forgedParent.Hash(), + Root: canonical.Root(), + Coinbase: common.Address{2}, + }) + if forged.Hash() == canonical.Hash() { + t.Fatalf("test setup: forged side block must differ from canonical") + } + // Sanity: the side block is a no-op vs its (forged) parent, the precise input + // the pre-hardening check would have wrongly exempted. + if chain.GetHeader(forged.ParentHash(), forged.NumberU64()-1).Root != forged.Root() { + t.Fatalf("test setup: forged side block must be a no-op vs its parent") + } + if !chain.isSidechainGhostState(forged, canonical) { + t.Fatalf("attack not detected: no-op side block at a state-changing canonical height must be rejected") + } +} + +// TestSidechainGhostStateMissingParentIsAttack covers the conservative fallback: +// when either parent header is unavailable, isSidechainGhostState cannot prove the +// blocks are genuine no-ops, so it must treat the match as an attack (return true). +func TestSidechainGhostStateMissingParentIsAttack(t *testing.T) { + chain, _, blocks, _, forkIdx, _ := setupPrunedGhostStateChain(t, rawdb.HashScheme, false) + + canonical := blocks[forkIdx] + + // A side block whose parent header is not in the database at all: GetHeader + // returns nil, so the exemption is denied without inspecting roots. + orphan := types.NewBlockWithHeader(&types.Header{ + Number: canonical.Number(), + ParentHash: common.Hash{0xde, 0xad}, + Root: canonical.Root(), + }) + if !chain.isSidechainGhostState(orphan, canonical) { + t.Fatalf("missing side parent must be treated as an attack") + } +} + // TestDeleteCreateRevert tests a weird state transition corner case that we hit // while changing the internals of statedb. The workflow is that a contract is // self destructed, then in a followup transaction (but same block) it's created