diff --git a/core/blockchain_impl.go b/core/blockchain_impl.go index f0e7e237ff..4757d3e5cc 100644 --- a/core/blockchain_impl.go +++ b/core/blockchain_impl.go @@ -2708,7 +2708,14 @@ func (bc *BlockChainImpl) CXMerkleProof(toShardID uint32, block *block.Header) ( func (bc *BlockChainImpl) WriteCXReceiptsProofSpent(db rawdb.DatabaseWriter, cxps []*types.CXReceiptsProof) error { for _, cxp := range cxps { - if cxp.Header != nil && bc.Config().IsCXMerkleProofReplayFixEpoch(cxp.Header.Epoch()) { + // Key the spent-marker off the signed Header, not the unauthenticated + // MerkleProof.ShardID/BlockNum: those fields are only bound to the + // Header by ValidateCXReceiptsProof from IsCXMerkleProofReplayFixEpoch + // onward, so a proof claiming an earlier epoch can carry a mutated + // MerkleProof while keeping a genuine Header/signature. Deriving the + // key from MerkleProof would let such a mutated copy of an already + //-applied receipt look unspent and be replayed for a fresh credit. + if cxp.Header != nil { if err := rawdb.WriteCXReceiptsProofSpentWithKey( db, cxp.Header.ShardID(), cxp.Header.Number().Uint64(), ); err != nil { @@ -2726,7 +2733,10 @@ func (bc *BlockChainImpl) WriteCXReceiptsProofSpent(db rawdb.DatabaseWriter, cxp func (bc *BlockChainImpl) IsSpent(cxp *types.CXReceiptsProof) bool { shardID := cxp.MerkleProof.ShardID blockNum := cxp.MerkleProof.BlockNum.Uint64() - if cxp.Header != nil && bc.Config().IsCXMerkleProofReplayFixEpoch(cxp.Header.Epoch()) { + // See WriteCXReceiptsProofSpent: always resolve the spent-marker key from + // the signed Header so the check can't be bypassed by mutating the + // unauthenticated MerkleProof fields on a genuine, previously-applied proof. + if cxp.Header != nil { shardID = cxp.Header.ShardID() blockNum = cxp.Header.Number().Uint64() } diff --git a/core/blockchain_impl_test.go b/core/blockchain_impl_test.go index ce3113f26d..1e23ee3bd1 100644 --- a/core/blockchain_impl_test.go +++ b/core/blockchain_impl_test.go @@ -11,6 +11,51 @@ import ( staking "github.com/harmony-one/harmony/staking/types" ) +// TestIsSpentIgnoresMutatedMerkleProofIdentity guards against replaying a +// genuine, already-applied CXReceiptsProof by mutating the unauthenticated +// MerkleProof.ShardID/BlockNum while keeping the same signed Header: the +// spent-marker must be keyed off the Header, which cannot be altered without +// invalidating the commit signature, not off MerkleProof fields that +// ValidateCXReceiptsProof only binds to the Header from +// IsCXMerkleProofReplayFixEpoch onward. +func TestIsSpentIgnoresMutatedMerkleProofIdentity(t *testing.T) { + key, _ := crypto.GenerateKey() + chain, _, header, database := getTestEnvironment(*key) + + header = header.With().ShardID(1).Number(big.NewInt(42)).Header() + + original := &types.CXReceiptsProof{ + Header: header, + MerkleProof: &types.CXMerkleProof{ + ShardID: 1, + BlockNum: big.NewInt(42), + }, + } + + batch := database.NewBatch() + if err := chain.WriteCXReceiptsProofSpent(batch, []*types.CXReceiptsProof{original}); err != nil { + t.Fatalf("WriteCXReceiptsProofSpent failed: %v", err) + } + if err := batch.Write(); err != nil { + t.Fatalf("batch.Write failed: %v", err) + } + + if !chain.IsSpent(original) { + t.Fatal("expected original proof to be marked spent") + } + + replay := &types.CXReceiptsProof{ + Header: header, // same genuine, signed header + MerkleProof: &types.CXMerkleProof{ + ShardID: 99, // mutated, unauthenticated + BlockNum: big.NewInt(9999), // mutated, unauthenticated + }, + } + if !chain.IsSpent(replay) { + t.Fatal("expected replay with mutated MerkleProof identity to be detected as already spent") + } +} + func TestPrepareStakingMetadata(t *testing.T) { key, _ := crypto.GenerateKey() chain, db, header, _ := getTestEnvironment(*key)