From 58b483a84196aa6e482d4b3b0b0441117a138de8 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 2 Dec 2019 20:34:30 -0500 Subject: [PATCH 1/4] fix: avoid panicing if we try to parse a non-sha multihash as a CIDv0 I'm pretty sure we never call `Parse` multihashes anyways, but we should be careful. --- cid.go | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/cid.go b/cid.go index b3c489b..60810eb 100644 --- a/cid.go +++ b/cid.go @@ -137,25 +137,39 @@ var CodecToStr = map[uint64]string{ DashTx: "dash-tx", } -// NewCidV0 returns a Cid-wrapped multihash. -// They exist to allow IPFS to work with Cids while keeping -// compatibility with the plain-multihash format used used in IPFS. -// NewCidV1 should be used preferentially. -func NewCidV0(mhash mh.Multihash) Cid { +// tryNewCidV0 tries to convert a multihash into a CIDv0 CID and returns an +// error on failure. +func tryNewCidV0(mhash mh.Multihash) (Cid, error) { // Need to make sure hash is valid for CidV0 otherwise we will // incorrectly detect it as CidV1 in the Version() method dec, err := mh.Decode(mhash) if err != nil { - panic(err) + return Undef, err } if dec.Code != mh.SHA2_256 || dec.Length != 32 { - panic("invalid hash for cidv0") + return Undef, fmt.Errorf("invalid hash for cidv0 %d-%d", dec.Code, dec.Length) } - return Cid{string(mhash)} + return Cid{string(mhash)}, nil +} + +// NewCidV0 returns a Cid-wrapped multihash. +// They exist to allow IPFS to work with Cids while keeping +// compatibility with the plain-multihash format used used in IPFS. +// NewCidV1 should be used preferentially. +// +// Panics if the multihash isn't sha2-256. +func NewCidV0(mhash mh.Multihash) Cid { + c, err := tryNewCidV0(mhash) + if err != nil { + panic(err) + } + return c } // NewCidV1 returns a new Cid using the given multicodec-packed // content type. +// +// Panics if the multihash is invalid. func NewCidV1(codecType uint64, mhash mh.Multihash) Cid { hashlen := len(mhash) // two 8 bytes (max) numbers plus hash @@ -203,7 +217,7 @@ func Parse(v interface{}) (Cid, error) { case []byte: return Cast(v2) case mh.Multihash: - return NewCidV0(v2), nil + return tryNewCidV0(v2) case Cid: return v2, nil default: @@ -234,7 +248,7 @@ func Decode(v string) (Cid, error) { return Undef, err } - return NewCidV0(hash), nil + return tryNewCidV0(hash) } _, data, err := mbase.Decode(v) From 08e15f8a6cdba3716b62c42782b09b6fe1b72716 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 2 Dec 2019 20:35:27 -0500 Subject: [PATCH 2/4] chore: avoid re-validated already validated CIDs --- builder.go | 2 +- cid.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builder.go b/builder.go index a168832..3d2fc77 100644 --- a/builder.go +++ b/builder.go @@ -38,7 +38,7 @@ func (p V0Builder) Sum(data []byte) (Cid, error) { if err != nil { return Undef, err } - return NewCidV0(hash), nil + return Cid{string(hash)}, nil } func (p V0Builder) GetCodec() uint64 { diff --git a/cid.go b/cid.go index 60810eb..2bb1e60 100644 --- a/cid.go +++ b/cid.go @@ -613,7 +613,7 @@ func CidFromBytes(data []byte) (int, Cid, error) { return 0, Undef, err } - return 34, NewCidV0(h), nil + return 34, Cid{string(h)}, nil } vers, n := binary.Uvarint(data) From 60ab0f84f0d7384ea282f149d7f96d73be40828f Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 2 Dec 2019 20:35:36 -0500 Subject: [PATCH 3/4] chore: avoid magic numbers --- cid.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cid.go b/cid.go index 2bb1e60..7150ecb 100644 --- a/cid.go +++ b/cid.go @@ -603,7 +603,7 @@ func PrefixFromBytes(buf []byte) (Prefix, error) { } func CidFromBytes(data []byte) (int, Cid, error) { - if len(data) > 2 && data[0] == 18 && data[1] == 32 { + if len(data) > 2 && data[0] == mh.SHA2_256 && data[1] == 32 { if len(data) < 34 { return 0, Undef, fmt.Errorf("not enough bytes for cid v0") } From 5df89959a068ade9f6c019467a5c3823122ed396 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 2 Dec 2019 20:41:59 -0500 Subject: [PATCH 4/4] test: test parsing non-sha256 hashes --- cid_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cid_test.go b/cid_test.go index 5d07497..9d0f9a1 100644 --- a/cid_test.go +++ b/cid_test.go @@ -584,3 +584,14 @@ func TestReadCidsFromBuffer(t *testing.T) { t.Fatal("had trailing bytes") } } + +func TestBadParse(t *testing.T) { + hash, err := mh.Sum([]byte("foobar"), mh.SHA3_256, -1) + if err != nil { + t.Fatal(err) + } + _, err = Parse(hash) + if err == nil { + t.Fatal("expected to fail to parse an invalid CIDv1 CID") + } +}