fix: accept legacy CommitHash and prevent mmap use-after-free for v6.3->v6.4 upgrade#3095
fix: accept legacy CommitHash and prevent mmap use-after-free for v6.3->v6.4 upgrade#3095masih wants to merge 2 commits intorelease/v6.4from
Conversation
…grade The Commit.Hash() algorithm changed in CON-76 (#2600) to include Height, Round, and BlockID in the Merkle tree alongside signatures. Blocks stored by v6.3 nodes have LastCommitHash computed with the old signatures-only algorithm. When v6.4 loads these blocks, ValidateBasic recomputes the hash with the new algorithm, causing a mismatch panic. Add Commit.LegacyHash() that uses the old algorithm, and fall back to it in ValidateBasic when the new hash doesn't match. Co-authored-by: Masih H. Derkani <m@derkani.org>
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/v6.4 #3095 +/- ##
=============================================
Coverage 58.38% 58.38%
=============================================
Files 2088 2088
Lines 172082 172099 +17
=============================================
+ Hits 100466 100481 +15
Misses 62659 62659
- Partials 8957 8959 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…-after-free When serving historical queries, the Query method opens a read-only memiavl instance (mmap'd), queries it, and defers Close(). The response byte slices (Key, Value) may point directly into the mmap'd region. When the deferred Close() unmaps the memory before JSON marshaling reads the response, the process segfaults (SIGSEGV). Copy res.Key and res.Value for non-latest queries so they survive the mmap unmap. Co-authored-by: Masih H. Derkani <m@derkani.org>
The Commit.Hash() algorithm was changed in CON-76 (#2600) to include Height, Round, and BlockID in the Merkle tree alongside signatures. Blocks stored by v6.3 nodes have LastCommitHash computed with the old signatures-only algorithm. When v6.4 loads these blocks during ABCI handshake replay, ValidateBasic recomputes the hash with the new algorithm, producing a different value and causing a panic first observed on `arctic-1`. Adds Commit.LegacyHash() that uses the old (signatures-only) algorithm, and updates Block.ValidateBasic() to fall back to it when the new hash doesn't match. Note that release 6.5 would make this fallback path entirely redundant, and it should be removed in the next release. Separated out from #3095 to capture a clear commit history of changes for the future archeologists.
The Commit.Hash() algorithm was changed in CON-76 (#2600) to include Height, Round, and BlockID in the Merkle tree alongside signatures. Blocks stored by v6.3 nodes have LastCommitHash computed with the old signatures-only algorithm. When v6.4 loads these blocks during ABCI handshake replay, ValidateBasic recomputes the hash with the new algorithm, producing a different value and causing a panic first observed on `arctic-1`. Adds Commit.LegacyHash() that uses the old (signatures-only) algorithm, and updates Block.ValidateBasic() to fall back to it when the new hash doesn't match. Note that release 6.5 would make this fallback path entirely redundant, and it should be removed in the next release. Separated out from #3095 to capture a clear commit history of changes for the future archeologists. (cherry picked from commit 512976d)
When serving historical queries, rootmulti.Store.Query() opens a read-only memiavl instance (mmap'd), queries it, and defers Close(). The response byte slices (Key, Value) may point directly into the mmap'd region. When the deferred Close() unmaps the memory before JSON marshaling reads the response, the process segfaults (observed on `arctic-1`. Copies res.Key and res.Value for non-latest queries so they survive the mmap unmap. Separated out from #3095 to capture a clear commit history of changes for the future archeologists.
When serving historical queries, rootmulti.Store.Query() opens a read-only memiavl instance (mmap'd), queries it, and defers Close(). The response byte slices (Key, Value) may point directly into the mmap'd region. When the deferred Close() unmaps the memory before JSON marshaling reads the response, the process segfaults (observed on `arctic-1`. Copies res.Key and res.Value for non-latest queries so they survive the mmap unmap. Separated out from #3095 to capture a clear commit history of changes for the future archeologists. (cherry picked from commit 145f854)
|
// File: sei-db/common/safe.go (recommended location) package common import "github.com/cosmos/cosmos-sdk/store/types" // SafeCopySlice creates a new independent copy of the input byte slice. // SafeCopyStoreValue is a convenience wrapper that copies both Key and Value // SafeCopyBytes is an alias for SafeCopySlice — use whichever name feels |
Describe your changes and provide context
Two fixes for deploying release/v6.4 on nodes previously running release/v6.3:
Fix 1: LastCommitHash mismatch panic on startup
The
Commit.Hash()algorithm was changed in CON-76 (#2600) to include Height, Round, and BlockID in the Merkle tree alongside signatures. Blocks stored by v6.3 nodes haveLastCommitHashcomputed with the old signatures-only algorithm. When v6.4 loads these blocks during ABCI handshake replay,ValidateBasicrecomputes the hash with the new algorithm, producing a different value and causing a panic:Adds
Commit.LegacyHash()that uses the old (signatures-only) algorithm, and updatesBlock.ValidateBasic()to fall back to it when the new hash doesn't match.Fix 2: SIGSEGV in historical RPC queries (use-after-free of mmap'd memory)
When serving historical queries,
rootmulti.Store.Query()opens a read-only memiavl instance (mmap'd), queries it, and defersClose(). The response byte slices (Key,Value) may point directly into the mmap'd region. When the deferredClose()unmaps the memory before JSON marshaling reads the response, the process segfaults:Copies
res.Keyandres.Valuefor non-latest queries so they survive the mmap unmap.Note on PR #3093
The IBC Upgrade Params fix from #3093 is already included in this branch since it was merged into
release/v6.4before this branch was created.Testing performed to validate your change
go test ./sei-tendermint/types/— all tests pass includingTestCommitHashandTestBlockValidateBasicgo test ./sei-tendermint/internal/store/— all store tests pass