From b77bb9e4aa09f57ddfa84ce4b4a3fe324db678f3 Mon Sep 17 00:00:00 2001 From: whyrusleeping Date: Sat, 16 Nov 2019 17:41:14 -0600 Subject: [PATCH] Audit pass for blatantly wrong panics --- api/permissioned.go | 4 ++-- chain/actors/actor_init.go | 2 +- chain/actors/actor_paych.go | 4 +++- chain/actors/actors.go | 4 ++-- chain/actors/actors_test.go | 2 +- chain/address/address.go | 6 +++--- chain/address/bench_test.go | 8 ++++---- chain/badtscache.go | 2 +- chain/blocksync/blocksync_client.go | 12 ++++++------ chain/gen/utils.go | 2 +- chain/sync.go | 3 ++- chain/types/bigint.go | 6 ++---- chain/types/blockheader.go | 4 ++-- chain/types/message.go | 2 +- chain/types/message_fuzz.go | 8 ++++---- chain/types/signature.go | 4 +--- chain/types/types_test.go | 2 +- chain/wallet/wallet.go | 3 ++- paych/state.go | 3 ++- 19 files changed, 41 insertions(+), 40 deletions(-) diff --git a/api/permissioned.go b/api/permissioned.go index 44a967707..eee6d330d 100644 --- a/api/permissioned.go +++ b/api/permissioned.go @@ -65,7 +65,7 @@ func permissionedAny(in interface{}, out interface{}) { field := rint.Type().Field(f) requiredPerm := Permission(field.Tag.Get("perm")) if requiredPerm == "" { - panic("missing 'perm' tag on " + field.Name) + panic("missing 'perm' tag on " + field.Name) // is this okay? can this be used to crash the process? } // Validate perm tag @@ -77,7 +77,7 @@ func permissionedAny(in interface{}, out interface{}) { } } if !ok { - panic("unknown 'perm' tag on " + field.Name) + panic("unknown 'perm' tag on " + field.Name) // is this okay? can this be used to crash the process? } fn := ra.MethodByName(field.Name) diff --git a/chain/actors/actor_init.go b/chain/actors/actor_init.go index 82b507d49..66d9d98fa 100644 --- a/chain/actors/actor_init.go +++ b/chain/actors/actor_init.go @@ -31,7 +31,7 @@ func init() { n, err := cbor.WrapObject(map[string]string{}, mh.SHA2_256, -1) if err != nil { - panic(err) + panic(err) // ok } EmptyCBOR = n.Cid() diff --git a/chain/actors/actor_paych.go b/chain/actors/actor_paych.go index 6d3e74039..3ff4dd95e 100644 --- a/chain/actors/actor_paych.go +++ b/chain/actors/actor_paych.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/ipfs/go-cid" + "github.com/minio/blake2b-simd" "github.com/filecoin-project/lotus/build" "github.com/filecoin-project/lotus/chain/actors/aerrors" @@ -94,7 +95,8 @@ type PCAUpdateChannelStateParams struct { } func hash(b []byte) []byte { - panic("blake 2b hash pls") + s := blake2b.Sum256(b) + return s[:] } type PaymentVerifyParams struct { diff --git a/chain/actors/actors.go b/chain/actors/actors.go index 10181eacd..fe111c125 100644 --- a/chain/actors/actors.go +++ b/chain/actors/actors.go @@ -24,7 +24,7 @@ var BurntFundsAddress = mustIDAddress(99) func mustIDAddress(i uint64) address.Address { a, err := address.NewIDAddress(i) if err != nil { - panic(err) + panic(err) // ok } return a } @@ -34,7 +34,7 @@ func init() { mustSum := func(s string) cid.Cid { c, err := pref.Sum([]byte(s)) if err != nil { - panic(err) + panic(err) // ok } return c } diff --git a/chain/actors/actors_test.go b/chain/actors/actors_test.go index 9e0c1186a..3433ea0d3 100644 --- a/chain/actors/actors_test.go +++ b/chain/actors/actors_test.go @@ -24,7 +24,7 @@ func blsaddr(n uint64) address.Address { addr, err := address.NewBLSAddress(buf) if err != nil { - panic(err) + panic(err) // ok } return addr diff --git a/chain/address/address.go b/chain/address/address.go index f06807757..4264371d3 100644 --- a/chain/address/address.go +++ b/chain/address/address.go @@ -91,7 +91,7 @@ func (a Address) Bytes() []byte { func (a Address) String() string { str, err := encode(Testnet, a) if err != nil { - panic(err) + panic(err) // I don't know if this one is okay } return str } @@ -314,12 +314,12 @@ func hash(ingest []byte, cfg *blake2b.Config) []byte { hasher, err := blake2b.New(cfg) if err != nil { // If this happens sth is very wrong. - panic(fmt.Sprintf("invalid address hash configuration: %v", err)) + panic(fmt.Sprintf("invalid address hash configuration: %v", err)) // ok } if _, err := hasher.Write(ingest); err != nil { // blake2bs Write implementation never returns an error in its current // setup. So if this happens sth went very wrong. - panic(fmt.Sprintf("blake2b is unable to process hashes: %v", err)) + panic(fmt.Sprintf("blake2b is unable to process hashes: %v", err)) // ok } return hasher.Sum(nil) } diff --git a/chain/address/bench_test.go b/chain/address/bench_test.go index 9556ae20c..2c21d549e 100644 --- a/chain/address/bench_test.go +++ b/chain/address/bench_test.go @@ -14,7 +14,7 @@ func blsaddr(n int64) Address { addr, err := NewBLSAddress(buf) if err != nil { - panic(err) + panic(err) // ok } return addr @@ -25,7 +25,7 @@ func makeActorAddresses(n int) [][]byte { for i := 0; i < n; i++ { a, err := NewActorAddress([]byte(fmt.Sprintf("ACTOR ADDRESS %d", i))) if err != nil { - panic(err) + panic(err) // ok } addrs = append(addrs, a.Bytes()) } @@ -50,7 +50,7 @@ func makeSecpAddresses(n int) [][]byte { a, err := NewSecp256k1Address(buf) if err != nil { - panic(err) + panic(err) // ok } addrs = append(addrs, a.Bytes()) @@ -64,7 +64,7 @@ func makeIDAddresses(n int) [][]byte { a, err := NewIDAddress(uint64(i)) if err != nil { - panic(err) + panic(err) // ok } addrs = append(addrs, a.Bytes()) diff --git a/chain/badtscache.go b/chain/badtscache.go index 8048a76d2..070375062 100644 --- a/chain/badtscache.go +++ b/chain/badtscache.go @@ -13,7 +13,7 @@ type BadBlockCache struct { func NewBadBlockCache() *BadBlockCache { cache, err := lru.NewARC(build.BadBlockCacheSize) if err != nil { - panic(err) + panic(err) // ok } return &BadBlockCache{ diff --git a/chain/blocksync/blocksync_client.go b/chain/blocksync/blocksync_client.go index 52e17f7a3..6a770f4b1 100644 --- a/chain/blocksync/blocksync_client.go +++ b/chain/blocksync/blocksync_client.go @@ -43,17 +43,17 @@ func NewBlockSyncClient(bserv dtypes.ChainBlockService, h host.Host) *BlockSync func (bs *BlockSync) processStatus(req *BlockSyncRequest, res *BlockSyncResponse) error { switch res.Status { case 101: // Partial Response - panic("not handled") + return xerrors.Errorf("not handling partial blocksync responses yet") case 201: // req.Start not found - return fmt.Errorf("not found") + return xerrors.Errorf("not found") case 202: // Go Away - panic("not handled") + return xerrors.Errorf("not handling 'go away' blocksync responses yet") case 203: // Internal Error - return fmt.Errorf("block sync peer errored: %s", res.Message) + return xerrors.Errorf("block sync peer errored: %s", res.Message) case 204: - return fmt.Errorf("block sync request invalid: %s", res.Message) + return xerrors.Errorf("block sync request invalid: %s", res.Message) default: - return fmt.Errorf("unrecognized response code: %d", res.Status) + return xerrors.Errorf("unrecognized response code: %d", res.Status) } } diff --git a/chain/gen/utils.go b/chain/gen/utils.go index 7bc830e7e..f60da262d 100644 --- a/chain/gen/utils.go +++ b/chain/gen/utils.go @@ -227,7 +227,7 @@ type GenMinerCfg struct { func mustEnc(i cbg.CBORMarshaler) []byte { enc, err := actors.SerializeParams(i) if err != nil { - panic(err) + panic(err) // ok } return enc } diff --git a/chain/sync.go b/chain/sync.go index 43c1030ff..316e2e0a7 100644 --- a/chain/sync.go +++ b/chain/sync.go @@ -95,7 +95,8 @@ const BootstrapPeerThreshold = 1 func (syncer *Syncer) InformNewHead(from peer.ID, fts *store.FullTipSet) { ctx := context.Background() if fts == nil { - panic("bad") + log.Errorf("got nil tipset in InformNewHead") + return } for _, b := range fts.Blocks { diff --git a/chain/types/bigint.go b/chain/types/bigint.go index 77f6f4643..85d2181c8 100644 --- a/chain/types/bigint.go +++ b/chain/types/bigint.go @@ -130,15 +130,13 @@ func (bi *BigInt) cborBytes() []byte { } switch { - case bi.Sign() == 0: - return []byte{} case bi.Sign() > 0: return append([]byte{0}, bi.Bytes()...) case bi.Sign() < 0: return append([]byte{1}, bi.Bytes()...) + default: // bi.Sign() == 0: + return []byte{} } - - panic("unreachable") } func fromCborBytes(buf []byte) (BigInt, error) { diff --git a/chain/types/blockheader.go b/chain/types/blockheader.go index d65a068d8..aa311db0c 100644 --- a/chain/types/blockheader.go +++ b/chain/types/blockheader.go @@ -66,7 +66,7 @@ func (b *BlockHeader) ToStorageBlock() (block.Block, error) { func (b *BlockHeader) Cid() cid.Cid { sb, err := b.ToStorageBlock() if err != nil { - panic(err) + panic(err) // Not sure i'm entirely comfortable with this one, needs to be checked } return sb.Cid() @@ -121,7 +121,7 @@ type MsgMeta struct { func (mm *MsgMeta) Cid() cid.Cid { b, err := mm.ToStorageBlock() if err != nil { - panic(err) + panic(err) // also maybe sketchy } return b.Cid() } diff --git a/chain/types/message.go b/chain/types/message.go index d64e017b8..d2bf27358 100644 --- a/chain/types/message.go +++ b/chain/types/message.go @@ -61,7 +61,7 @@ func (m *Message) ToStorageBlock() (block.Block, error) { func (m *Message) Cid() cid.Cid { b, err := m.ToStorageBlock() if err != nil { - panic(fmt.Sprintf("failed to marshal message: %s", err)) + panic(fmt.Sprintf("failed to marshal message: %s", err)) // I think this is maybe sketchy, what happens if we try to serialize a message with an undefined address in it? } return b.Cid() diff --git a/chain/types/message_fuzz.go b/chain/types/message_fuzz.go index f6d7180b0..4ef5f6ba2 100644 --- a/chain/types/message_fuzz.go +++ b/chain/types/message_fuzz.go @@ -12,19 +12,19 @@ func FuzzMessage(data []byte) int { } reData, err := msg.Serialize() if err != nil { - panic(err) + panic(err) // ok } var msg2 Message err = msg2.UnmarshalCBOR(bytes.NewReader(data)) if err != nil { - panic(err) + panic(err) // ok } reData2, err := msg.Serialize() if err != nil { - panic(err) + panic(err) // ok } if !bytes.Equal(reData, reData2) { - panic("reencoding not equal") + panic("reencoding not equal") // ok } return 1 } diff --git a/chain/types/signature.go b/chain/types/signature.go index b82d552f0..f3a61327b 100644 --- a/chain/types/signature.go +++ b/chain/types/signature.go @@ -55,10 +55,8 @@ func (s *Signature) TypeCode() int { return IKTSecp256k1 case KTBLS: return IKTBLS - case "": - return IKTUnknown default: - panic("unsupported signature type") + return IKTUnknown } } diff --git a/chain/types/types_test.go b/chain/types/types_test.go index 6bd4c0883..4849db5b7 100644 --- a/chain/types/types_test.go +++ b/chain/types/types_test.go @@ -14,7 +14,7 @@ func blsaddr(n int64) address.Address { addr, err := address.NewBLSAddress(buf) if err != nil { - panic(err) + panic(err) // ok } return addr diff --git a/chain/wallet/wallet.go b/chain/wallet/wallet.go index 34df4d4aa..eeb1af1cb 100644 --- a/chain/wallet/wallet.go +++ b/chain/wallet/wallet.go @@ -2,6 +2,7 @@ package wallet import ( "context" + "fmt" "sort" "strings" "sync" @@ -72,7 +73,7 @@ func (w *Wallet) Sign(ctx context.Context, addr address.Address, msg []byte) (*t }, nil default: - panic("cant do it sir") + return nil, fmt.Errorf("cannot sign with unsupported key type: %q", ki.Type) } } diff --git a/paych/state.go b/paych/state.go index 4785d547f..8a0150620 100644 --- a/paych/state.go +++ b/paych/state.go @@ -7,6 +7,7 @@ import ( "github.com/filecoin-project/lotus/chain/actors" "github.com/filecoin-project/lotus/chain/address" "github.com/filecoin-project/lotus/chain/types" + xerrors "golang.org/x/xerrors" ) func (pm *Manager) loadPaychState(ctx context.Context, ch address.Address) (*types.Actor, *actors.PaymentChannelActorState, error) { @@ -52,7 +53,7 @@ func (pm *Manager) laneState(ctx context.Context, ch address.Address, lane uint6 for _, v := range vouchers { for range v.Voucher.Merges { - panic("merges todo") // TODO: nonce check + return actors.LaneState{}, xerrors.Errorf("paych merges not handled yet") } if v.Voucher.Lane != lane {