v1.0.1: audit fixes — fetchCertRaw status check, .part cleanup, AVK guards, strict merkle, JSON error envelope
Independent code audit (in-repo, fresh-eyes pass) flagged 0 critical, 4
high, 8 medium, 7 low. This commit addresses all 4 highs + the JSON
error-path inconsistency + the vestigial verify.STM stub.
HIGH fixes:
- cmd/mithril-go/main.go fetchCertRaw: missing status check let HTML 4xx/5xx
bodies fall through to confusing JSON-decode errors. Added explicit
StatusCode>=400 check + 16 MiB response body cap + Accept header.
- internal/artifact/download.go: SHA mismatch left .part on disk, causing
every retry to resume the corrupted bytes and fail SHA forever. Now
removes .part on hash mismatch so the next attempt starts clean.
- internal/stm/types.go DecodeAVK: rejects total_stake=0 and nr_leaves=0
at decode-time. internal/stm/lottery.go adds defensive guard for
stake==0 || totalStake==0 to prevent big.Rat.SetFrac panic (DoS vector
for the MCP server when fed crafted AVK).
- internal/stm/merkle.go: now requires (a) every proof value is exactly
32 bytes, (b) indices are STRICTLY ascending (no duplicates),
(c) every index is < nr_leaves, (d) all proof values are consumed by
the algorithm. Prevents parser-differential bugs vs upstream Rust.
JSON error-path wiring:
- cmd/mithril-go/json.go: replaced unused emitJSONErr with failure() helper
that routes errors to stdout-as-JSON when -json is set, else stderr-as-text.
Error envelope shape: {error: {code, kind, message}} where 'kind' is a
stable short string (network/integrity/verify/usage/internal) for agents
to branch on without parsing human text.
- All -json-supporting commands (info, list, show, cert, verify+subcommands)
now use failure() in error paths instead of bare fmt.Fprintln(stderr).
- Verified: 'verify -json deadbeef' on a bogus hash now emits valid JSON
to stdout with exit=3, instead of empty stdout + text on stderr.
Vestigial code:
- internal/verify/verify.go: removed STM() stub + ErrSTMNotImplemented.
Real STM verification has lived in internal/stm/verify.go since the
crypto sprint; the stub was dead code from milestone-by-milestone work.
Verification (still all green):
- preprod chain: 90 certs, 1124 wins ✓
- mainnet head: 59 signers, 1972 wins ✓
- preprod head: 2 signers, 11 wins ✓
- preprod genesis: Ed25519 ✓
- JSON error envelope on bogus hash: well-formed JSON, exit=3
- internal/stm unit test: PASS
Audit findings deferred to v1.0.2+: bubble-sort in stm.Verify (medium,
perf only at scale); int-vs-uint64 truncation guards on 32-bit targets
(medium, won't bite on 64-bit); tar mode-bit masking (medium, low impact
since archives are from trusted aggregator); no User-Agent header on
aggregator requests (low, op nicety); MCP scanner silent stop on >10 MiB
line (low, defensive).
This commit is contained in:
parent
e9557ca05b
commit
9d6c7cffbe
7 changed files with 123 additions and 82 deletions
|
|
@ -121,6 +121,10 @@ func Download(ctx context.Context, uri, destPath, expectedSHA256 string, progres
|
|||
if expectedSHA256 != "" {
|
||||
got := hex.EncodeToString(h.Sum(nil))
|
||||
if got != expectedSHA256 {
|
||||
// Remove the .part file — leaving it behind would cause every
|
||||
// subsequent retry to resume from the same corrupted bytes and
|
||||
// fail SHA again indefinitely.
|
||||
_ = os.Remove(partPath)
|
||||
return fmt.Errorf("SHA256 mismatch: want %s, got %s", expectedSHA256, got)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -58,6 +58,12 @@ func IsLotteryWon(phiF float64, ev [64]byte, stake, totalStake uint64) bool {
|
|||
if math.Abs(phiF-1.0) < 1e-15 {
|
||||
return true
|
||||
}
|
||||
// Defensive: zero-stake or zero-total-stake produces nonsense (and
|
||||
// totalStake==0 would panic at SetFrac). Guard at the lottery layer
|
||||
// in addition to AVK-decode-time validation.
|
||||
if stake == 0 || totalStake == 0 {
|
||||
return false
|
||||
}
|
||||
|
||||
// ev as big int (LE interpretation)
|
||||
evInt := evAsBigInt(ev)
|
||||
|
|
|
|||
|
|
@ -4,7 +4,6 @@ import (
|
|||
"bytes"
|
||||
"encoding/binary"
|
||||
"fmt"
|
||||
"sort"
|
||||
|
||||
"golang.org/x/crypto/blake2b"
|
||||
)
|
||||
|
|
@ -78,16 +77,31 @@ func VerifyMerkleBatch(root []byte, nrLeaves int, leafValues [][]byte, indices [
|
|||
if len(leafValues) != len(indices) {
|
||||
return fmt.Errorf("leaves/indices count mismatch: %d vs %d", len(leafValues), len(indices))
|
||||
}
|
||||
// Must be sorted ascending
|
||||
if nrLeaves <= 0 {
|
||||
return fmt.Errorf("nrLeaves must be positive, got %d", nrLeaves)
|
||||
}
|
||||
// Validate every proof node is a 32-byte BLAKE2b-256 digest. Anything
|
||||
// shorter or longer is malformed and Rust would reject it.
|
||||
for i, v := range proofValues {
|
||||
if len(v) != 32 {
|
||||
return fmt.Errorf("proof value [%d]: got %d bytes, want 32", i, len(v))
|
||||
}
|
||||
}
|
||||
// Indices must be strictly ascending — duplicates would create
|
||||
// double-claiming under the same leaf and the algorithm doesn't expect
|
||||
// them. (Rust uses sort_unstable + equality compare against the input;
|
||||
// equivalent to "non-decreasing" but doesn't reject equal-adjacent.
|
||||
// We're stricter than upstream here on purpose.)
|
||||
ordered := make([]int, len(indices))
|
||||
for i, v := range indices {
|
||||
if v >= uint64(nrLeaves) {
|
||||
return fmt.Errorf("index [%d]=%d out of range (nr_leaves=%d)", i, v, nrLeaves)
|
||||
}
|
||||
ordered[i] = int(v)
|
||||
}
|
||||
sortedCopy := append([]int(nil), ordered...)
|
||||
sort.Ints(sortedCopy)
|
||||
for i := range ordered {
|
||||
if ordered[i] != sortedCopy[i] {
|
||||
return fmt.Errorf("indices not sorted ascending: %v", indices)
|
||||
for i := 1; i < len(ordered); i++ {
|
||||
if ordered[i] <= ordered[i-1] {
|
||||
return fmt.Errorf("indices not strictly ascending at [%d]: %v", i, indices)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -155,6 +169,12 @@ func VerifyMerkleBatch(root []byte, nrLeaves int, leafValues [][]byte, indices [
|
|||
if len(currentLayer) != 1 {
|
||||
return fmt.Errorf("verification ended with %d nodes, want 1", len(currentLayer))
|
||||
}
|
||||
// All proof values must be consumed. Trailing bytes mean the proof
|
||||
// shipped extra nodes the algorithm didn't need — likely malformed
|
||||
// or attacker-padded.
|
||||
if len(values) > 0 {
|
||||
return fmt.Errorf("proof has %d unconsumed values — malformed", len(values))
|
||||
}
|
||||
if !bytes.Equal(currentLayer[0], root) {
|
||||
return fmt.Errorf("root mismatch: got %x, want %x", currentLayer[0], root)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -116,6 +116,12 @@ func DecodeAVK(rawJSON []byte) (*AVK, error) {
|
|||
if len(wire.MTCommitment.Root) != 32 {
|
||||
return nil, fmt.Errorf("AVK root: got %d bytes, want 32", len(wire.MTCommitment.Root))
|
||||
}
|
||||
if wire.TotalStake == 0 {
|
||||
return nil, fmt.Errorf("AVK total_stake is zero")
|
||||
}
|
||||
if wire.MTCommitment.NrLeaves == 0 {
|
||||
return nil, fmt.Errorf("AVK nr_leaves is zero")
|
||||
}
|
||||
return &AVK{
|
||||
MerkleRoot: wire.MTCommitment.Root,
|
||||
NumLeaves: wire.MTCommitment.NrLeaves,
|
||||
|
|
|
|||
|
|
@ -27,7 +27,6 @@ var (
|
|||
ErrNotGenesis = errors.New("certificate is not a genesis certificate")
|
||||
ErrBadSignature = errors.New("genesis signature verification failed")
|
||||
ErrSignedMessageHash = errors.New("signed_message does not match SHA256(protocol_message)")
|
||||
ErrSTMNotImplemented = errors.New("STM signature verification not implemented yet")
|
||||
)
|
||||
|
||||
// The Mithril enum order on ProtocolMessagePartKey — BTreeMap iteration
|
||||
|
|
@ -182,9 +181,5 @@ func GenesisFromJSON(verifyKey ed25519.PublicKey, signedMessageHex, genesisSigna
|
|||
return Genesis(verifyKey, signedMessageHex, genesisSignatureHex, pm)
|
||||
}
|
||||
|
||||
// STM verifies a non-genesis certificate's aggregate BLS signature.
|
||||
// Stub — target is Mithril STM paper §5 (signing) + §6 (aggregation)
|
||||
// using gnark-crypto's bls12-381 primitives.
|
||||
func STM(protocolMessageJSON, multiSignature []byte, avk any) error {
|
||||
return ErrSTMNotImplemented
|
||||
}
|
||||
// STM verification lives in the sibling internal/stm package — see
|
||||
// stm.Verify(). This file is genesis-Ed25519-only.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue