From 1cb007e8d3fa5c7bc00603bb4882921a4c14fd05 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 9 Jan 2018 14:38:10 -0800 Subject: [PATCH 1/2] ensure we don't store arbitrary data Explicitly clean DHT records before storing them. --- handlers.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/handlers.go b/handlers.go index d9ee9f285aa..4d5e77a5c7f 100644 --- a/handlers.go +++ b/handlers.go @@ -152,6 +152,18 @@ func (dht *IpfsDHT) checkLocalDatastore(k string) (*recpb.Record, error) { return rec, nil } +// Cleans the record (to avoid storing arbitrary data). +func cleanRecord(rec *recpb.Record) { + rec.XXX_unrecognized = nil + rec.TimeReceived = nil + + // Only include the author if there's a signature (otherwise, it's + // unvalidated and could be anything). + if len(rec.Signature) == 0 { + rec.Author = nil + } +} + // Store a value in this peer local storage func (dht *IpfsDHT) handlePutValue(ctx context.Context, p peer.ID, pmes *pb.Message) (_ *pb.Message, err error) { eip := log.EventBegin(ctx, "handlePutValue", p) @@ -169,6 +181,7 @@ func (dht *IpfsDHT) handlePutValue(ctx context.Context, p peer.ID, pmes *pb.Mess log.Infof("Got nil record from: %s", p.Pretty()) return nil, errors.New("nil record") } + cleanRecord(rec) if err = dht.verifyRecordLocally(rec); err != nil { log.Warningf("Bad dht record in PUT from: %s. %s", peer.ID(pmes.GetRecord().GetAuthor()), err) From 817715374830ef8df54f900e0109f881cd8dfb69 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 11 Jan 2018 10:01:51 -0800 Subject: [PATCH 2/2] add test case for cleanRecord function --- handlers_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 handlers_test.go diff --git a/handlers_test.go b/handlers_test.go new file mode 100644 index 00000000000..9d931700507 --- /dev/null +++ b/handlers_test.go @@ -0,0 +1,65 @@ +package dht + +import ( + "bytes" + "testing" + + proto "github.com/gogo/protobuf/proto" + recpb "github.com/libp2p/go-libp2p-record/pb" +) + +func TestCleanRecordSigned(t *testing.T) { + actual := new(recpb.Record) + actual.TimeReceived = proto.String("time") + actual.XXX_unrecognized = []byte("extra data") + actual.Signature = []byte("signature") + actual.Author = proto.String("author") + actual.Value = []byte("value") + actual.Key = proto.String("key") + + cleanRecord(actual) + actualBytes, err := proto.Marshal(actual) + if err != nil { + t.Fatal(err) + } + + expected := new(recpb.Record) + expected.Signature = []byte("signature") + expected.Author = proto.String("author") + expected.Value = []byte("value") + expected.Key = proto.String("key") + expectedBytes, err := proto.Marshal(expected) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(actualBytes, expectedBytes) { + t.Error("failed to clean record") + } +} + +func TestCleanRecord(t *testing.T) { + actual := new(recpb.Record) + actual.TimeReceived = proto.String("time") + actual.XXX_unrecognized = []byte("extra data") + actual.Key = proto.String("key") + actual.Value = []byte("value") + + cleanRecord(actual) + actualBytes, err := proto.Marshal(actual) + if err != nil { + t.Fatal(err) + } + + expected := new(recpb.Record) + expected.Key = proto.String("key") + expected.Value = []byte("value") + expectedBytes, err := proto.Marshal(expected) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(actualBytes, expectedBytes) { + t.Error("failed to clean record") + } +}