PAXB - Audit

CertiK2 Nov 2023Files named in the report

Audit

CertiK reviewed PAXB (PAXB - Audit) on 2 Nov 2023: 17 findings, 0 critical; 16 marked fixed, 1 open.

No open critical findings

Open the report Open PAXB’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings17

Statuses are the security firm’s own, from its report.

Severity	Open	Fixed
Critical	0	0
High	0	0
Medium	0	0
Low	1	16

Low`BlockReceipts` Parameter Encoding Issue In API:Fixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - ethclient/ethclient.go Both block number and block hash should be OK as parameter in `eth_getBlockReceipts`. ```go=111 func (ec *Client) BlockReceipts(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) ([]*types.Receipt, error) { var r []*types.Receipt err := ec.c.CallContext(ctx, &r, "eth_getBlockReceipts", blockNrOrHash) if err == nil && r == nil { return nil, ethereum.NotFound } return r, err } ``` In the method `BlockReceipts` to handle the API `eth_getBlockReceipts`, after marshaling `rpc.BlockNumberOrHash` we have an json string rather than the string format, in result an invalid response could return.

Low`eth_getProof` Returns Invalid Code Hash With EmpFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - internal/ethapi/api.go ```go // GetProof returns the Merkle-proof for a given account and optionally some storage keys. func (s *BlockChainAPI) GetProof(ctx context.Context, address common.Address, storageKeys []string, blockNrOrHash rpc.BlockNumberOrHash) (*AccountResult, error) { ``` When `eth_getProof` is called for an account with an empty storage trie (storageTrie == nil), it always returns the empty code hash, even if the code is not empty. Unconditionally looks up the `codehash` in the `GetProof` function would return the invalid one.

LowBootnodes Config Priority For Bootnodes FlagFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - cmd/utils/flags.go The bootnodes are used for starting the `geth` node in the P2P networks, and they could be set in below aspects: - --`bootnodes` flag - `config.toml` file When it comes to the priority order for bootnode configuration, the command (cmd) flag should take precedence over the configuration file. However, in some cases, when attempting to restart a node using the bootnode flag, it appears to be ignored.

LowCompact DB Missing Key Starts With `0xff`Fixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - internal/ethapi/api.go ```go func (api *DebugAPI) ChaindbCompact() error { for b := byte(0); b < 255; b++ { log.Info("Compacting chain database", "range", fmt.Sprintf("0x%0.2X-0x%0.2X", b, b+1)) if err := api.b.ChainDb().Compact([]byte{b}, []byte{b + 1}); err != nil { ``` As the old for loop for `b := byte(0); b < 255; b++ {` will only iterate the keys from `(0x00, 0x01)` to `(0xfe, 0xff)`, so the keys after `0xff` were not compacted.

LowDeadlock When Matcher Session Hits An ErrorFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - core/bloombits/matcher.go A deadlock situation can arise when a pruned node receives an eth_getLogs request for pruned blocks, and the go-ethereum application has not pruned any historical data. This issue may not be reproducible in geth due to its pruning strategy. However, the root cause is evident and can be traced back to the core/bloombits module in the repository. Implementing this patch would help prevent any unknown problems in the future. Therefore, it is highly recommended to address this issue in the codebase to ensure stability and prevent potential complications.

LowDevp2p Discv4 `crawl` Does NOT WORKFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - cmd/devp2p/crawl.go When crawling `geth` nodes without specific inputs, it should crawl the predefined `bootnodes`. However, it did not even printout any useful info at all. ```go func newCrawler(input nodeSet, disc resolver, iters ...enode.Iterator) *crawler { c := &crawler{ input: input, output: make(nodeSet, len(input)), disc: disc, iters: iters, inputIter: enode.IterNodes(input.nodes()), ch: make(chan *enode.Node), closed: make(chan struct{}), } c.iters = append(c.iters, c.inputIter) // Copy input to output initially. Any nodes that fail validation // will be dropped from output during the run. for id, n := range input { c.output[id] = n } return c } ``` There is any validation on the input in the `newCrawler` function and nothing error could be printed to indicate the situation.

LowEnsure DB Is Closed Before The Geth Client ExitFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - cmd/geth/chaincmd.go - cmd/geth/dbcmd.go - cmd/geth/snapshot.go - cmd/geth/verkle.go Before exiting the geth application using command lines from the mentioned files, it is recommended to close the database to prevent any potential storage collisions.

LowFreezer Repair is Disallowed When Database is inFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - cmd/geth/chaincmd.go - cmd/utils/flags.go - core/rawdb/accessors_trie.go - eth/backend.go - eth/ethconfig/config.go When unclean shutdown happens and freezer tables need to be repair. In database read-only mode, the freezer will only be validated but not repair and terminates the node abnormally as belows. ```bash INFO [10-10|09:06:03.021] Starting Geth on Ethereum mainnet... INFO [10-10|09:06:03.023] Maximum peer count ETH=250 LES=0 total=250 INFO [10-10|09:06:03.026] Smartcard socket not found, disabling err="stat /run/pcscd/pcscd.comm: no such file or directory" WARN [10-10|09:06:03.029] Sanitizing cache to Go's GC limits provided=16384 updated=10584 INFO [10-10|09:06:04.161] Using pebble as the backing database INFO [10-10|09:06:04.161] Allocated cache and file handles database=/root/.ethereum/geth/chaindata cache=5.17GiB handles=524,288 Chain metadata databaseVersion: 8 (0x8) headBlockHash: 0xe58eb5e1327a7b0b34629dc598c51432df4a9a307be7ca5332901cbcb38be9b6 headFastBlockHash: 0xe58eb5e1327a7b0b34629dc598c51432df4a9a307be7ca5332901cbcb38be9b6 headHeaderHash: 0xe58eb5e1327a7b0b34629dc598c51432df4a9a307be7ca5332901cbcb38be9b6 lastPivotNumber: <nil> len(snapshotSyncStatus): 0 bytes snapshotDisabled: false snapshotJournal: 34 bytes snapshotRecoveryNumber: 11323955 (0xacca33) snapshotRoot: 0x657a381cafd20db4beb6f9020ad7fd011a9b7d91bc21e5b0b57d09e35ee0d369 txIndexTail: 6609620 (0x64dad4) fastTxLookupLimit: <nil> SkeletonSyncStatus: {"Subchains":[{"Head":18290416,"Tail":14185576,"Next":"0x5df719e7f8df2f927564984f0d8285a06a5e826b8c6b78cc63fdb7ce3b56039b"}],"Finalized":18290339} Fatal: Could not open database: freezer tables bodies and hashes have differing head: 14745785 != 14745687 ```

LowInaccurate Reference to Difficulty FieldFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - cmd/evm/internal/t8ntool/block.go Per the function context, the reference to `Difficulty` field should be `i.Header.Difficulty` instead of `header.Difficulty`.

LowMake Rhs-proof Align With Last Key In Range ProofFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - trie/proof.go - core/state/snapshot/generate.go - eth/protocols/snap/sync.go During the snap-sync period, fetching the main account was split up and requested in multiple stages. An origin field is used. The left-hand side of the proof relates to the `origin`, and is free-standing from the first leaf. On the other hand, (pun intended), the right-hand side, there's no such 'gap' between "along what path does the proof walk" and the last provided leaf. The proof must prove the last element (unless there are no elements). Therefore, the function trie.VerifyRangeProof() can be simplified by removing an argument due to the upcoming stacktrie-based verifier that does not support such fancy features as standalone right-hand borders.

Show 6 more

LowSnap Sync Failure On Empty Storage RangeFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - eth/protocols/snap/sync.go On the method `OnStorage`, snap sync is taken with storage range. Currently, the response to an empty storage range is discarded. However, this response is valid when there are no available storage slots within that range. ```go=2549 func (s *Syncer) OnStorage(peer SyncPeer, id uint64, hashes [][]common.Hash, slots [][][]byte, proof [][]byte) error { for _, hashset := range hashes { size += common.StorageSize(common.HashLength * len(hashset)) hashCount += len(hashset) } for _, slotset := range slots { for _, slot := range slotset { size += common.StorageSize(len(slot)) } slotCount += len(slotset) } for _, node := range proof { size += common.StorageSize(len(node)) } logger := peer.Log().New("reqid", id) logger.Trace("Delivering ranges of storage slots", "accounts", len(hashes), "hashes", hashCount, "slots", slotCount, "proofs", len(proof), "size", size) // Whether or not the response is valid, we can mark the peer as idle and // notify the scheduler to assign a new task. If the response is invalid, // we'll drop the peer in a bit. defer func() { s.lock.Lock() defer s.lock.Unlock() if _, ok := s.peers[peer.ID()]; ok { s.storageIdlers[peer.ID()] = struct{}{} } select { case s.update <- struct{}{}: default: } }() s.lock.Lock() // Ensure the response is for a valid request req, ok := s.storageReqs[id] if !ok { // Request stale, perhaps the peer timed out but came through in the end logger.Warn("Unexpected storage ranges packet") s.lock.Unlock() return nil } delete(s.storageReqs, id) s.rates.Update(peer.ID(), StorageRangesMsg, time.Since(req.time), int(size)) // Clean up the request timeout timer, we'll see how to proceed further based // on the actual delivered content if !req.timeout.Stop() { // The timeout is already triggered, and this request will be reverted+rescheduled s.lock.Unlock() return nil } // Reject the response if the hash sets and slot sets don't match, or if the // peer sent more data than requested. if len(hashes) != len(slots) { s.lock.Unlock() s.scheduleRevertStorageRequest(req) // reschedule request logger.Warn("Hash and slot set size mismatch", "hashset", len(hashes), "slotset", len(slots)) return errors.New("hash and slot set size mismatch") } if len(hashes) > len(req.accounts) { s.lock.Unlock() s.scheduleRevertStorageRequest(req) // reschedule request logger.Warn("Hash set larger than requested", "hashset", len(hashes), "requested", len(req.accounts)) return errors.New("hash set larger than requested") } // Response is valid, but check if peer is signalling that it does not have // the requested data. For storage range queries that means the state being // retrieved was either already pruned remotely, or the peer is not yet // synced to our head. if len(hashes) == 0 { logger.Debug("Peer rejected storage request") s.statelessPeers[peer.ID()] = struct{}{} s.lock.Unlock() s.scheduleRevertStorageRequest(req) // reschedule request return nil } ``` If the response is discarded, it is possible that the finalization of the last range will indeed be skipped, which could potentially halt the entire sync process indefinitely.

LowThe Block Parameter In `eth_call` Should Be LateFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - internal/ethapi/api.go In the specification of the JSON RPC API, the base block parameter is optional. However, in practical situations, "latest" is commonly considered a reasonable default that most people would use. ```go=1161 func (s *BlockChainAPI) Call(ctx context.Context, args TransactionArgs, blockNrOrHash rpc.BlockNumberOrHash, overrides *StateOverride, blockOverrides *BlockOverrides) (hexutil.Bytes, error) { result, err := DoCall(ctx, s.b, args, blockNrOrHash, overrides, blockOverrides, s.b.RPCEVMTimeout(), s.b.RPCGasCap()) ``` In the call method of the API handling, it is recommended to set the corresponding block parameter to latest as the default value. Reference: - [`Json RPC API|ethereum.org`](https://ethereum.org/en/developers/docs/apis/json-rpc/#spec)

LowTyposFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - core/rawdb/database.go - eth/backend.go - cmd/utils/flags.go - eth/downloader/skeleton.go - core/txpool/legacypool/core/txpool/legacypool/legacypool.go.go - core/txpool/validation.go - core/txpool/blobpool/evictheap.go - core/txpool/blobpool/limbo.go - core/txpool/blobpool/priority.go - core/txpool/errors.go - core/txpool/txpool.go - cmd/evm/testdata/8/readme.md - eth/ethconfig/config.go - params/config.go There are typos in the files of the codebase.

LowUnderpriced Transactions Should Be RebroadcastedFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - eth/fetcher/tx_fetcher.go In situations where the miner's pool is empty and the gas price has decreased, it is generally expected that all transactions with sufficient price and correct nonce will be mined. However, there are cases where certain transactions get stuck in the p2p node's pool indefinitely after experiencing a traffic jam. Even if the gas price in the blockchain decreases, these transactions may not be processed unless they are replaced by other transactions with a relatively lower price. In the method `Enqueue` for handling the transaction queue, there is a threshold of `maxTxUnderpricedSetSize` for the `underpriced` transactions. ```go func (f *TxFetcher) Enqueue(peer string, txs []*types.Transaction, direct bool) error { for j, err := range f.addTxs(batch) { // Track the transaction hash if the price is too low for us. // Avoid re-request this transaction when we receive another // announcement. if errors.Is(err, txpool.ErrUnderpriced) || errors.Is(err, txpool.ErrReplaceUnderpriced) { for f.underpriced.Cardinality() >= maxTxUnderpricedSetSize { f.underpriced.Pop() } f.underpriced.Add(batch[j].Hash()) } ``` However, in these situations, there is no action taken to handle the underpriced transactions when the gas price hike subsides. This can still have an impact on the user experience, as these transactions may still remain unresolved or pending.

LowUpdate `blobGasUsed` In Chainmaker When Tx Is AddFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - core/chain_makers.go The `gasUsed` field in the header is automatically updated when a transaction is added. Instead of requiring the caller to manually set the gas used, it is advisable to update the blob's gas used automatically.

LowUse Read Lock in `readonly` ModeFixed

Commits: - [`fca847da37de126b6a0652af813ec810a3e3d6ea`](https://github.com/paxb-paul/paxb-net/tree/fca847da37de126b6a0652af813ec810a3e3d6ea) Files: - core/rawdb/freezer.go When the db is in `readyonly` mode, using read lock instead of read-write lock can improve speed when reading ancient data in a shareable EFS.