@cryptotaxi247 / kubo / commits / 8f362d2b1

dht: protect against a panic in case record on pbmessage is nil

License: MIT Signed-off-by: Jeromy <why@ipfs.io>

Jeromy committed Sep 8, 2016 at 12:21 UTC 8f362d2b1500809cbae2ee08c2b42a6f226a10b8
3 files changed +26 -5
routing/dht/dht_test.go
+15 -2
@@ -11,14 +11,15 @@ import (
11
12 key "github.com/ipfs/go-ipfs/blocks/key"
13 routing "github.com/ipfs/go-ipfs/routing"
14 + pb "github.com/ipfs/go-ipfs/routing/dht/pb"
15 record "github.com/ipfs/go-ipfs/routing/record"
16 ci "github.com/ipfs/go-ipfs/thirdparty/testutil/ci"
17 travisci "github.com/ipfs/go-ipfs/thirdparty/testutil/ci/travis"
17 - ds "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore"
18 - dssync "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore/sync"
18
19 pstore "gx/ipfs/QmQdnfvZQuhdT93LNc5bos52wAmdr3G2p6G8teLJMEN32P/go-libp2p-peerstore"
20 peer "gx/ipfs/QmRBqJF7hb8ZSpRcMwUt8hNhydWcxGEhtk81HKq6oUwKvs/go-libp2p-peer"
21 + ds "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore"
22 + dssync "gx/ipfs/QmTxLSvdhwg68WJimdS6icLPhZi28aTp6b7uihC2Yb47Xk/go-datastore/sync"
23 netutil "gx/ipfs/QmVCe3SNMjkcPgnpFhZs719dheq6xE7gJwjzV7aWcUM4Ms/go-libp2p/p2p/test/util"
24 ma "gx/ipfs/QmYzDkkgAEmrcNzFCiYo6L1dTX4EAG1gZkbtdbd9trL4vd/go-multiaddr"
25 u "gx/ipfs/QmZNVWh8LLjAavuQ2JXuFmuYH3C11xo988vSgp7UQrTRj1/go-ipfs-util"
@@ -826,3 +827,15 @@ func TestConnectCollision(t *testing.T) {
827 dhtB.host.Close()
828 }
829 }
830 +
831 +func TestBadProtoMessages(t *testing.T) {
832 + ctx, cancel := context.WithCancel(context.Background())
833 + defer cancel()
834 +
835 + d := setupDHT(ctx, t)
836 +
837 + nilrec := new(pb.Message)
838 + if _, err := d.handlePutValue(ctx, "testpeer", nilrec); err == nil {
839 + t.Fatal("should have errored on nil record")
840 + }
841 +}
routing/dht/handlers.go
+7 -3
@@ -150,13 +150,17 @@ func (dht *IpfsDHT) handlePutValue(ctx context.Context, p peer.ID, pmes *pb.Mess
150 defer log.EventBegin(ctx, "handlePutValue", p).Done()
151 dskey := key.Key(pmes.GetKey()).DsKey()
152
153 - if err := dht.verifyRecordLocally(pmes.GetRecord()); err != nil {
153 + rec := pmes.GetRecord()
154 + if rec == nil {
155 + log.Infof("Got nil record from: %s", p.Pretty())
156 + return nil, errors.New("nil record")
157 + }
158 +
159 + if err := dht.verifyRecordLocally(rec); err != nil {
160 log.Warningf("Bad dht record in PUT from: %s. %s", key.Key(pmes.GetRecord().GetAuthor()), err)
161 return nil, err
162 }
163
158 - rec := pmes.GetRecord()
159 -
164 // record the time we receive every record
165 rec.TimeReceived = proto.String(u.FormatRFC3339(time.Now()))
166
routing/dht/records.go
+4
@@ -107,6 +107,10 @@ func (dht *IpfsDHT) getPublicKeyFromNode(ctx context.Context, p peer.ID) (ci.Pub
107 // verifyRecordLocally attempts to verify a record. if we do not have the public
108 // key, we fail. we do not search the dht.
109 func (dht *IpfsDHT) verifyRecordLocally(r *pb.Record) error {
110 + if r == nil {
111 + log.Error("nil record passed into verifyRecordLocally")
112 + return fmt.Errorf("nil record")
113 + }
114
115 if len(r.Signature) > 0 {
116 // First, validate the signature