From 484939d2745f1f3480d393e63ac96d4100b5102b Mon Sep 17 00:00:00 2001 From: Darren Kelly Date: Mon, 8 Jan 2024 13:16:45 +0000 Subject: [PATCH] fix: some minor issues and add gosec --- .github/workflows/go-lint.yaml | 13 +++++++++++- api/subscriptions/subscriptions.go | 30 +++++++++++++++++++++++----- blake2b/blake2x.go | 10 ++++++++-- block/block.go | 10 +++++++++- block/header.go | 5 ++++- cmd/thor/main.go | 32 ++++++++++++++++++++++++++---- cmd/thor/node/packer_loop.go | 3 ++- cmd/thor/utils.go | 18 +++++++++++++---- comm/sync.go | 5 ++++- muxdb/muxdb.go | 10 +++++++++- p2psrv/rpc/rpc.go | 5 ++++- 11 files changed, 119 insertions(+), 22 deletions(-) diff --git a/.github/workflows/go-lint.yaml b/.github/workflows/go-lint.yaml index b24bb29c1..adac1fd86 100644 --- a/.github/workflows/go-lint.yaml +++ b/.github/workflows/go-lint.yaml @@ -11,7 +11,7 @@ permissions: jobs: golangci: - name: lint + name: golangci-lint runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -48,3 +48,14 @@ jobs: # Optional: The mode to install golangci-lint. It can be 'binary' or 'goinstall'. # install-mode: "goinstall" + + gosec: + name: gosec-lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Run Gosec Security Scanner + uses: securego/gosec@master + with: + args: ./... diff --git a/api/subscriptions/subscriptions.go b/api/subscriptions/subscriptions.go index ab8cda0d6..9497dbb2b 100644 --- a/api/subscriptions/subscriptions.go +++ b/api/subscriptions/subscriptions.go @@ -248,7 +248,10 @@ func (s *Subscriptions) handlePendingTransactions(w http.ResponseWriter, req *ht case <-closed: return nil case <-pingTicker.C: - conn.WriteMessage(websocket.PingMessage, nil) + err := conn.WriteMessage(websocket.PingMessage, nil) + if err != nil { + return nil + } } } } @@ -264,9 +267,18 @@ func (s *Subscriptions) setupConn(w http.ResponseWriter, req *http.Request) (*we s.wg.Add(1) go func() { defer s.wg.Done() - conn.SetReadDeadline(time.Now().Add(pongWait)) + err := conn.SetReadDeadline(time.Now().Add(pongWait)) + + if err != nil { + return + } + conn.SetPongHandler(func(string) error { - conn.SetReadDeadline(time.Now().Add(pongWait)) + err := conn.SetReadDeadline(time.Now().Add(pongWait)) + + if err != nil { + return nil + } return nil }) for { @@ -319,7 +331,11 @@ func (s *Subscriptions) pipe(conn *websocket.Conn, reader msgReader, closed chan case <-closed: return nil case <-pingTicker.C: - conn.WriteMessage(websocket.PingMessage, nil) + err := conn.WriteMessage(websocket.PingMessage, nil) + + if err != nil { + return nil + } default: } } else { @@ -330,7 +346,11 @@ func (s *Subscriptions) pipe(conn *websocket.Conn, reader msgReader, closed chan return nil case <-ticker.C(): case <-pingTicker.C: - conn.WriteMessage(websocket.PingMessage, nil) + err := conn.WriteMessage(websocket.PingMessage, nil) + + if err != nil { + return nil + } } } } diff --git a/blake2b/blake2x.go b/blake2b/blake2x.go index 52c414db0..0acb6d442 100644 --- a/blake2b/blake2x.go +++ b/blake2b/blake2x.go @@ -144,7 +144,10 @@ func (x *xof) Read(p []byte) (n int, err error) { x.nodeOffset++ x.d.initConfig(&x.cfg) - x.d.Write(x.root[:]) + n, err = x.d.Write(x.root[:]) + if err != nil { + return + } x.d.finalize(&x.block) copy(p, x.block[:]) @@ -160,7 +163,10 @@ func (x *xof) Read(p []byte) (n int, err error) { x.nodeOffset++ x.d.initConfig(&x.cfg) - x.d.Write(x.root[:]) + n, err = x.d.Write(x.root[:]) + if err != nil { + return + } x.d.finalize(&x.block) x.offset = copy(p, x.block[:todo]) diff --git a/block/block.go b/block/block.go index 29e12913f..0540eb06a 100644 --- a/block/block.go +++ b/block/block.go @@ -7,6 +7,7 @@ package block import ( "fmt" + "github.com/inconshreveable/log15" "io" "sync/atomic" @@ -19,6 +20,10 @@ const ( ComplexSigSize = 81 + 65 ) +var ( + log = log15.New("pkg", "block") +) + // Block is an immutable block type. type Block struct { header *Header @@ -100,7 +105,10 @@ func (b *Block) Size() metric.StorageSize { return cached.(metric.StorageSize) } var size metric.StorageSize - rlp.Encode(&size, b) + err := rlp.Encode(&size, b) + if err != nil { + log.Warn("failed to encode block", "err", err) + } b.cache.size.Store(size) return size } diff --git a/block/header.go b/block/header.go index b5b51b29b..2c70ed61c 100644 --- a/block/header.go +++ b/block/header.go @@ -137,7 +137,7 @@ func (h *Header) SigningHash() (hash thor.Bytes32) { defer func() { h.cache.signingHash.Store(hash) }() return thor.Blake2bFn(func(w io.Writer) { - rlp.Encode(w, []interface{}{ + err := rlp.Encode(w, []interface{}{ &h.body.ParentID, h.body.Timestamp, h.body.GasLimit, @@ -150,6 +150,9 @@ func (h *Header) SigningHash() (hash thor.Bytes32) { &h.body.StateRoot, &h.body.ReceiptsRoot, }) + if err != nil { + log.Warn("failed to encode block header", "err", err) + } }) } diff --git a/cmd/thor/main.go b/cmd/thor/main.go index f18150fc9..6975b5897 100644 --- a/cmd/thor/main.go +++ b/cmd/thor/main.go @@ -152,7 +152,13 @@ func defaultAction(ctx *cli.Context) error { if err != nil { return err } - defer func() { log.Info("closing main database..."); mainDB.Close() }() + defer func() { + log.Info("closing main database...") + err := mainDB.Close() + if err != nil { + log.Warn("failed to close main database", "err", err) + } + }() skipLogs := ctx.Bool(skipLogsFlag.Name) @@ -160,7 +166,13 @@ func defaultAction(ctx *cli.Context) error { if err != nil { return err } - defer func() { log.Info("closing log database..."); logDB.Close() }() + defer func() { + log.Info("closing log database...") + err := logDB.Close() + if err != nil { + log.Warn("failed to close log database", "err", err) + } + }() repo, err := initChainRepository(gene, mainDB, logDB) if err != nil { @@ -261,11 +273,23 @@ func soloAction(ctx *cli.Context) error { if mainDB, err = openMainDB(ctx, instanceDir); err != nil { return err } - defer func() { log.Info("closing main database..."); mainDB.Close() }() + defer func() { + log.Info("closing main database...") + err := mainDB.Close() + if err != nil { + log.Warn("failed to close main database", "err", err) + } + }() if logDB, err = openLogDB(ctx, instanceDir); err != nil { return err } - defer func() { log.Info("closing log database..."); logDB.Close() }() + defer func() { + log.Info("closing log database...") + err := logDB.Close() + if err != nil { + log.Warn("failed to close log database", "err", err) + } + }() } else { instanceDir = "Memory" mainDB = openMemMainDB() diff --git a/cmd/thor/node/packer_loop.go b/cmd/thor/node/packer_loop.go index 60730fe35..0dedfcbf9 100644 --- a/cmd/thor/node/packer_loop.go +++ b/cmd/thor/node/packer_loop.go @@ -153,7 +153,8 @@ func (n *Node) pack(flow *packer.Flow) error { // write logs if logEnabled { - if n.writeLogs(newBlock, receipts, oldBest.Header.ID()); err != nil { + err := n.writeLogs(newBlock, receipts, oldBest.Header.ID()) + if err != nil { return errors.Wrap(err, "write logs") } } diff --git a/cmd/thor/utils.go b/cmd/thor/utils.go index 9765705c5..51475cba1 100644 --- a/cmd/thor/utils.go +++ b/cmd/thor/utils.go @@ -146,7 +146,11 @@ func handleXGenesisID(h http.Handler, genesisID thor.Bytes32) http.Handler { } w.Header().Set(headerKey, expectedID) if actualID != "" && actualID != expectedID { - io.Copy(ioutil.Discard, r.Body) + _, err := io.Copy(io.Discard, r.Body) + if err != nil { + http.Error(w, "", http.StatusInternalServerError) + return + } http.Error(w, "genesis id mismatch", http.StatusForbidden) return } @@ -429,7 +433,7 @@ func newP2PComm(ctx *cli.Context, repo *chain.Repository, txPool *txpool.TxPool, } nat, err := nat.Parse(ctx.String(natFlag.Name)) if err != nil { - cli.ShowAppHelp(ctx) + err := cli.ShowAppHelp(ctx) return nil, errors.Wrap(err, "parse -nat flag") } @@ -521,10 +525,16 @@ func startAPIServer(ctx *cli.Context, handler http.Handler, genesisID thor.Bytes srv := &http.Server{Handler: handler} var goes co.Goes goes.Go(func() { - srv.Serve(listener) + err := srv.Serve(listener) + if err != nil { + log.Warn("API server stopped", "err", err) + } }) return "http://" + listener.Addr().String() + "/", func() { - srv.Close() + err := srv.Close() + if err != nil { + log.Warn("failed to close API server", "err", err) + } goes.Wait() }, nil } diff --git a/comm/sync.go b/comm/sync.go index 9fe69d3d8..8a69f2466 100644 --- a/comm/sync.go +++ b/comm/sync.go @@ -84,7 +84,10 @@ func warmupBlocks(ctx context.Context, fetched <-chan []*block.Block, warmedUp c h := blk.Header() queue <- func() { h.ID() - h.Beta() + _, err := h.Beta() + if err != nil { + return + } } for _, tx := range blk.Transactions() { tx := tx diff --git a/muxdb/muxdb.go b/muxdb/muxdb.go index 1b0da3971..459a4cabe 100644 --- a/muxdb/muxdb.go +++ b/muxdb/muxdb.go @@ -10,6 +10,7 @@ package muxdb import ( "context" "encoding/json" + "github.com/inconshreveable/log15" "github.com/syndtr/goleveldb/leveldb" dberrors "github.com/syndtr/goleveldb/leveldb/errors" @@ -29,6 +30,10 @@ const ( namedStoreSpace = byte(3) // the key space for named store. ) +var ( + log = log15.New("pkg", "muxdb") +) + const ( propStoreName = "muxdb.props" configKey = "config" @@ -105,7 +110,10 @@ func Open(path string, options *Options) (*MuxDB, error) { DedupedPtnFactor: options.TrieDedupedPartitionFactor, } if err := cfg.LoadOrSave(propStore); err != nil { - ldb.Close() + closeErr := ldb.Close() + if closeErr != nil { + log.Warn("failed to close leveldb", "err", closeErr) + } return nil, err } diff --git a/p2psrv/rpc/rpc.go b/p2psrv/rpc/rpc.go index a7d6fdecf..cad424d2f 100644 --- a/p2psrv/rpc/rpc.go +++ b/p2psrv/rpc/rpc.go @@ -113,7 +113,10 @@ func (r *RPC) Serve(handleFunc HandleFunc, maxMsgSize uint32) error { } else { if err := handleFunc(&msg, func(result interface{}) { if callID != 0 { - p2p.Send(r.rw, msg.Code, &msgData{callID, true, result}) + sendError := p2p.Send(r.rw, msg.Code, &msgData{callID, true, result}) + if sendError != nil { + log.Warn("failed to send result", "err", sendError) + } } // here we skip result for Notify (callID == 0) }); err != nil {