diff --git a/cl/persistence/base_encoding/rabbit.go b/cl/persistence/base_encoding/rabbit.go index cd07e9773c3..3e9a94b9892 100644 --- a/cl/persistence/base_encoding/rabbit.go +++ b/cl/persistence/base_encoding/rabbit.go @@ -113,6 +113,7 @@ func ReadRabbits(out []uint64, r io.Reader) ([]uint64, error) { if err != nil { return nil, err } + // overflow_false_positive if current+count < current { return nil, fmt.Errorf("rabbit: index overflow at current=%d count=%d", current, count) } diff --git a/cl/phase1/core/state/cache.go b/cl/phase1/core/state/cache.go index 3e68b6dbd62..6d9156ce5a5 100644 --- a/cl/phase1/core/state/cache.go +++ b/cl/phase1/core/state/cache.go @@ -242,6 +242,7 @@ func (b *CachingBeaconState) _refreshActiveBalancesIfNeeded() { *b.totalActiveBalanceCache = 0 // Check global cache using block root at beginning of previous epoch + // overflow_false_positive blockRootAtBegginingPrevEpoch, err := b.GetBlockRootAtSlot(((epoch - 1) * b.BeaconConfig().SlotsPerEpoch) - 1) if err == nil { if _, cachedBalance, ok := caches.ActiveValidatorsCacheGlobal.Get(epoch, blockRootAtBegginingPrevEpoch); ok && cachedBalance != 0 { diff --git a/cl/phase1/core/state/cache_accessors.go b/cl/phase1/core/state/cache_accessors.go index d90f6002c08..fc445135471 100644 --- a/cl/phase1/core/state/cache_accessors.go +++ b/cl/phase1/core/state/cache_accessors.go @@ -48,6 +48,7 @@ func (b *CachingBeaconState) GetActiveValidatorsIndices(epoch uint64) []uint64 { } // Check global cache using block root at beginning of previous epoch + // overflow_false_positive blockRootAtBegginingPrevEpoch, err := b.GetBlockRootAtSlot(((epoch - 1) * b.BeaconConfig().SlotsPerEpoch) - 1) if err == nil { if cachedIndicies, _, ok := caches.ActiveValidatorsCacheGlobal.Get(epoch, blockRootAtBegginingPrevEpoch); ok && len(cachedIndicies) > 0 { diff --git a/cl/phase1/network/backward_beacon_downloader.go b/cl/phase1/network/backward_beacon_downloader.go index ef7cfad7315..6b71bcccd5e 100644 --- a/cl/phase1/network/backward_beacon_downloader.go +++ b/cl/phase1/network/backward_beacon_downloader.go @@ -199,6 +199,7 @@ func (b *BackwardBeaconDownloader) RequestMore(ctx context.Context) error { // Falls back to the beacon API when P2P is unavailable and an HTTP URL is configured. func (b *BackwardBeaconDownloader) fetchBlockRange(ctx context.Context) ([]*cltypes.SignedBeaconBlock, error) { const count = uint64(64) + // overflow_false_positive start := b.slotToDownload.Load() - count + 1 if start > b.slotToDownload.Load() { // overflow check start = 0 diff --git a/common/bitutil/select.go b/common/bitutil/select.go index abe831ea6cb..c3ad1d663ec 100644 --- a/common/bitutil/select.go +++ b/common/bitutil/select.go @@ -83,6 +83,21 @@ const ( * */ +// wmul, wadd and wsub are this package's deliberately modular arithmetic. The +// wraparound is load-bearing, so it is expressed here once rather than at each +// use — the same operations Rust spells wrapping_mul/wrapping_add/wrapping_sub. +// Package-level -overflowdetect exemption does not cover these, because they +// are inlined into instrumented callers. + +// overflow_false_positive +func wmul(a, b uint64) uint64 { return a * b } + +// overflow_false_positive +func wadd(a, b uint64) uint64 { return a + b } + +// overflow_false_positive +func wsub(a, b uint64) uint64 { return a - b } + func Select64(x uint64, k int) (place int) { /* Original implementation - a bit obfuscated to satisfy Golang's inlining costs s := x @@ -91,16 +106,16 @@ func Select64(x uint64, k int) (place int) { s = (s + (s >> 4)) & (0xF * kOnesStep8) byteSums := s * kOnesStep8 */ - s := x - ((x & kOnesStep4xA) >> 1) - s = (s & kOnesStep4x3) + ((s >> 2) & kOnesStep4x3) - byteSums := ((s + (s >> 4)) & kOnesStep8xF) * kOnesStep8 + s := wsub(x, (x&kOnesStep4xA)>>1) + s = wadd(s&kOnesStep4x3, (s>>2)&kOnesStep4x3) + byteSums := wmul(wadd(s, s>>4)&kOnesStep8xF, kOnesStep8) /* Original implementation: kStep8 := uint64(k) * kOnesStep8 geqKStep8 := ((kStep8 | kLAMBDAsStep8) - byteSums) & kLAMBDAsStep8 place = bits.OnesCount64(geqKStep8) * 8 - byteRank := uint64(k) - (((byteSums << 8) >> place) & uint64(0xFF)) + byteRank := wsub(uint64(k), ((byteSums<<8)>>place)&uint64(0xFF)) */ - place = bits.OnesCount64((((uint64(k)*kOnesStep8)|kLAMBDAsStep8)-byteSums)&kLAMBDAsStep8) * 8 - byteRank := uint64(k) - (((byteSums << 8) >> place) & uint64(0xFF)) + place = bits.OnesCount64(wsub(wmul(uint64(k), kOnesStep8)|kLAMBDAsStep8, byteSums)&kLAMBDAsStep8) * 8 + byteRank := wsub(uint64(k), ((byteSums<<8)>>place)&uint64(0xFF)) return place + int(kSelectInByte[((x>>place)&0xFF)|(byteRank<<8)]) } diff --git a/db/kv/kvcache/cache.go b/db/kv/kvcache/cache.go index b90ec3b7421..866a4d5f6ef 100644 --- a/db/kv/kvcache/cache.go +++ b/db/kv/kvcache/cache.go @@ -238,6 +238,7 @@ func (c *Coherent) advanceRoot(stateVersionID uint64) (r *CoherentRoot) { c.roots[stateVersionID] = r } + // overflow_false_positive if prevView, ok := c.roots[stateVersionID-1]; ok && prevView.isCanonical { //log.Info("advance: clone", "from", viewID-1, "to", viewID) r.cache = prevView.cache.Copy() diff --git a/db/recsplit/eliasfano32/elias_fano.go b/db/recsplit/eliasfano32/elias_fano.go index 8a6f53c008b..131c67e1598 100644 --- a/db/recsplit/eliasfano32/elias_fano.go +++ b/db/recsplit/eliasfano32/elias_fano.go @@ -736,6 +736,7 @@ func (efi *EliasFanoIter) decrement() { // note: there can be an underflow here after the last Next() // but that is ok since we are protected from ErrEliasFanoIterExhausted + // overflow_false_positive efi.lowerIdx -= efi.l efi.upperMask >>= 1 } diff --git a/db/recsplit/recsplit.go b/db/recsplit/recsplit.go index e96c76b4091..d18c47fa4b2 100644 --- a/db/recsplit/recsplit.go +++ b/db/recsplit/recsplit.go @@ -82,9 +82,18 @@ func newExistenceFilterWriter(filePath string, v version.DataStructureVersion) ( * @return a 64-bit integer obtained by mixing the bits of `z`. */ +// wmul, wadd and wsub are this package's deliberately modular arithmetic. The +// wraparound is load-bearing, so it is expressed here once rather than at each +// use — the same operations Rust spells wrapping_mul/wrapping_add/wrapping_sub. +// Package-level -overflowdetect exemption does not cover these, because they +// are inlined into instrumented callers. + +// overflow_false_positive +func wmul(a, b uint64) uint64 { return a * b } + func remix(z uint64) uint64 { - z = (z ^ (z >> 30)) * 0xbf58476d1ce4e5b9 - z = (z ^ (z >> 27)) * 0x94d049bb133111eb + z = wmul(z^(z>>30), 0xbf58476d1ce4e5b9) + z = wmul(z^(z>>27), 0x94d049bb133111eb) return z ^ (z >> 31) } diff --git a/db/seg/patricia/aho_corasick.go b/db/seg/patricia/aho_corasick.go index 787be56e2a5..f8b1e9f9a4f 100644 --- a/db/seg/patricia/aho_corasick.go +++ b/db/seg/patricia/aho_corasick.go @@ -295,11 +295,23 @@ const ( // predictable iterations instead of one data-dependent branch per label. // labels carries eight bytes of tail padding to keep the last word in bounds; // a hit in that padding, or in the next state's labels, lands at k >= hi. +// wmul, wadd and wsub are this package's deliberately modular arithmetic. The +// wraparound is load-bearing, so it is expressed here once rather than at each +// use — the same operations Rust spells wrapping_mul/wrapping_add/wrapping_sub. +// Package-level -overflowdetect exemption does not cover these, because they +// are inlined into instrumented callers. + +// overflow_false_positive +func wmul(a, b uint64) uint64 { return a * b } + +// overflow_false_positive +func wsub(a, b uint64) uint64 { return a - b } + func swarEdge(labels []byte, children []int32, lo, hi int32, b byte) int32 { - bcast := uint64(b) * swarOnes + bcast := wmul(uint64(b), swarOnes) for i := lo; i < hi; i += 8 { v := binary.LittleEndian.Uint64(labels[i:]) ^ bcast - if z := (v - swarOnes) &^ v & swarHighs; z != 0 { + if z := wsub(v, swarOnes) &^ v & swarHighs; z != 0 { // borrows only propagate up, so the lowest flagged byte is a real hit if k := i + int32(bits.TrailingZeros64(z)>>3); k < hi { return children[k] diff --git a/execution/commitment/prefix_trie.go b/execution/commitment/prefix_trie.go index 9f624f7b992..007d1d1b301 100644 --- a/execution/commitment/prefix_trie.go +++ b/execution/commitment/prefix_trie.go @@ -92,6 +92,7 @@ func popcount(n *prefixNode) int { func childIndex(n *prefixNode, nib byte) (int, bool) { mask := uint16(1) << nib + // overflow_false_positive idx := bits.OnesCount16(n.bitmap & (mask - 1)) return idx, n.bitmap&mask != 0 } diff --git a/execution/protocol/evm.go b/execution/protocol/evm.go index 5db55af831f..219541f1d50 100644 --- a/execution/protocol/evm.go +++ b/execution/protocol/evm.go @@ -120,6 +120,7 @@ func NewEVMTxContext(msg Message) evmtypes.TxContext { // GetHashFn returns a GetHashFunc which retrieves header hashes by number func GetHashFn(ref *types.Header, getHeader func(hash common.Hash, number uint64) (*types.Header, error)) func(n uint64) (common.Hash, error) { + // overflow_false_positive refNumber := ref.Number.Uint64() - 1 refHash := ref.ParentHash lastKnownNumber := refNumber diff --git a/execution/protocol/txn_executor.go b/execution/protocol/txn_executor.go index b2d485ee680..4b091402762 100644 --- a/execution/protocol/txn_executor.go +++ b/execution/protocol/txn_executor.go @@ -304,6 +304,7 @@ func (st *TxnExecutor) preCheck(gasBailout bool, intrinsicGasResult mdgas.Intrin return upfrontTxnFees{}, &nonceError{err: ErrNonceTooHigh, from: from, txNonce: msgNonce, stateNonce: stNonce} } else if stNonce > msgNonce { return upfrontTxnFees{}, &nonceError{err: ErrNonceTooLow, from: from, txNonce: msgNonce, stateNonce: stNonce} + // overflow_false_positive } else if stNonce+1 < stNonce { return upfrontTxnFees{}, fmt.Errorf("%w: address %v, nonce: %d", ErrNonceMax, from, stNonce) diff --git a/execution/stagedsync/stage_senders.go b/execution/stagedsync/stage_senders.go index 59e85b4b9bc..e39a03c86cc 100644 --- a/execution/stagedsync/stage_senders.go +++ b/execution/stagedsync/stage_senders.go @@ -332,6 +332,7 @@ Loop: if err := s.Update(tx, to); err != nil { return err } + // overflow_false_positive log.Debug(fmt.Sprintf("[%s] Recovery done", logPrefix), "from", startFrom, "to", to, "blocks", to-startFrom+1, "took", time.Since(recoveryStart)) } return nil diff --git a/execution/types/transaction_signing.go b/execution/types/transaction_signing.go index 708a87f0c9f..d5626cbb195 100644 --- a/execution/types/transaction_signing.go +++ b/execution/types/transaction_signing.go @@ -418,6 +418,7 @@ func recoverPlain(context *secp256k1.Context, sighash common.Hash, r, s, vb *uin if vb.BitLen() > 8 { return accounts.NilAddress, ErrInvalidSig } + // overflow_false_positive vByte := byte(vb.Uint64() - 27) if !crypto.TransactionSignatureIsValid(vByte, r, s, !homestead) { return accounts.NilAddress, ErrInvalidSig diff --git a/execution/vm/analysis.go b/execution/vm/analysis.go index 125adfc7af2..44b652a7a8a 100644 --- a/execution/vm/analysis.go +++ b/execution/vm/analysis.go @@ -47,6 +47,7 @@ func codeBitmap(code []byte) bitvec { if pc+8 <= codeLen { w := binary.LittleEndian.Uint64(code[pc : pc+8]) t := (w & swarPushHi) ^ swarPushPat + // overflow_false_positive if (t-swarLow)&^t&swarHigh == 0 { // no PUSH byte in this word pc += 8 continue diff --git a/execution/vm/evm.go b/execution/vm/evm.go index d1f340fdc09..d4bf3a62da2 100644 --- a/execution/vm/evm.go +++ b/execution/vm/evm.go @@ -230,6 +230,7 @@ func (evm *EVM) handleFrameRevert(gasRemaining *mdgas.MdGas, err error, snapshot // and subtracting it wraps mod 2^64 into the correct positive sum. Safe at // any gas magnitude. func deriveFrameExecutionGasUsed(inputTotal, gasRemainingTotal uint64, stateGasUsed int64) uint64 { + // overflow_false_positive return inputTotal - gasRemainingTotal - uint64(stateGasUsed) } diff --git a/misc/gosentry-run.sh b/misc/gosentry-run.sh new file mode 100755 index 00000000000..ca0d1c86dac --- /dev/null +++ b/misc/gosentry-run.sh @@ -0,0 +1,69 @@ +#!/usr/bin/env bash +# Build and run erigon under the gosentry toolchain, which panics on integer +# overflow/underflow at runtime. https://github.com/trailofbits/gosentry +# +# Two kinds of deliberate wraparound have to be silenced or the node cannot +# start: +# +# * whole packages whose purpose is modular arithmetic (below) — exempted +# here, because marking every operation would mean 40 markers in murmur3 +# alone; +# * single sites inside packages worth instrumenting — marked in the source +# with `// overflow_false_positive`, so the rest of the package keeps its +# checks. +# +# Anything that panics after that is a finding. +set -euo pipefail + +GOSENTRY=${GOSENTRY:-$HOME/fzz/gosentry} +P=github.com/erigontech/erigon + +# entire package is modular arithmetic by design +EXEMPT=( + "$P/common/murmur3" # MurmurHash3 mixing + "$P/common/crypto/blake2b" # BLAKE2b compression + "$P/common/bitutil" # SWAR select/popcount + "$P/db/seg/patricia" # SWAR edge lookup + "$P/db/recsplit" # RecSplit remix + "$P/execution/protocol/rules/ethash" # FNV +) + +flags=() +for pkg in "${EXEMPT[@]}"; do + flags+=("-gcflags=$pkg=-overflowdetect=false") +done + +export GOTOOLCHAIN=local CGO_ENABLED=1 + +case "${1:-build}" in +build) + # -truncationdetect=true is deliberately NOT set: it fires within the first + # few executions of nearly every package, so it needs its own triage pass. + target=${2:-./cmd/erigon} + out=${OUT:-./build/bin/$(basename "$target")-gosentry} + exec "$GOSENTRY/bin/go" build "${flags[@]}" -o "$out" "$target" + ;; +test) + shift + exec "$GOSENTRY/bin/go" test "${flags[@]}" -short -count=1 "${@:-./...}" + ;; +fuzz) + # the LibAFL harness is linked by cargo, which does not pull in the C++ + # runtime that evmone's modexp needs + export RUSTFLAGS="-C link-arg=-lstdc++" + pkg=$2 target=$3 dur=${4:-60s} + # Overflow detection off by default here: hunting crashes and hunting + # overflow are different jobs, and leaving it on makes every campaign stall + # on the first deliberate wraparound instead of exploring. OVERFLOW=1 turns + # it back on for an overflow-specific run. + if [ "${OVERFLOW:-0}" != "1" ]; then + flags+=("-gcflags=all=-overflowdetect=false") + fi + exec "$GOSENTRY/bin/go" test "$pkg" -run='^$' -fuzz="^$target\$" -fuzztime="$dur" \ + --focus-on-new-code=false --catch-races=false --catch-leaks=false "${flags[@]}" + ;; +*) + echo "usage: $0 {build [pkg]|test [pkgs...]|fuzz [dur]}" >&2 + exit 2 + ;; +esac diff --git a/p2p/enode/nodedb.go b/p2p/enode/nodedb.go index fe6f2ca11f4..4291a979db2 100644 --- a/p2p/enode/nodedb.go +++ b/p2p/enode/nodedb.go @@ -595,6 +595,7 @@ func (db *DB) QuerySeeds(n int, maxAge time.Duration) []*Node { // of hitting all existing nodes in very small databases. ctr := id[0] rand.Read(id[:]) + // overflow_false_positive id[0] = ctr + id[0]%16 var n *Node for k, v, err := c.Seek(nodeKey(id)); k != nil && n == nil; k, v, err = c.Next() {