From 832b6a0170dbce4b9895b2d17fbab2857d95426c Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 17 Nov 2016 16:51:59 +0100 Subject: [PATCH 1/9] Add basic fuzz test and basic corpus --- cid_fuzz.go | 17 +++++++++++++++++ fuzz-data/corpus/cid0 | 1 + fuzz-data/corpus/cid1 | 1 + 3 files changed, 19 insertions(+) create mode 100644 cid_fuzz.go create mode 100644 fuzz-data/corpus/cid0 create mode 100644 fuzz-data/corpus/cid1 diff --git a/cid_fuzz.go b/cid_fuzz.go new file mode 100644 index 0000000..495ec4d --- /dev/null +++ b/cid_fuzz.go @@ -0,0 +1,17 @@ +// +build gofuzz + +package cid + +func Fuzz(data []byte) int { + cid, err := Cast(data) + + if err != nil { + return 0 + } + + _ = cid.Bytes() + if !cid.Equals(cid) { + panic("inequality") + } + return 1 +} diff --git a/fuzz-data/corpus/cid0 b/fuzz-data/corpus/cid0 new file mode 100644 index 0000000..56fd786 --- /dev/null +++ b/fuzz-data/corpus/cid0 @@ -0,0 +1 @@ + gD1e-D/q3~(7`8n \ No newline at end of file diff --git a/fuzz-data/corpus/cid1 b/fuzz-data/corpus/cid1 new file mode 100644 index 0000000..e0420b6 --- /dev/null +++ b/fuzz-data/corpus/cid1 @@ -0,0 +1 @@ +q -[ïh[ (ΰ[)D \ No newline at end of file From 8aeb1a44a81e6e76ec0cf59f769263f035b8240d Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 17 Nov 2016 18:23:54 +0100 Subject: [PATCH 2/9] Handle UVarit overflows --- cid.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/cid.go b/cid.go index 68f1c01..2fd43e7 100644 --- a/cid.go +++ b/cid.go @@ -3,6 +3,7 @@ package cid import ( "bytes" "encoding/binary" + "errors" "fmt" "strings" @@ -88,6 +89,23 @@ func Decode(v string) (*Cid, error) { return Cast(data) } +var ( + ErrVarintBuffSmall = errors.New("reading varint: buffer to small") + ErrVarintTooBig = errors.New("reading varint: varint bigger than 64bits" + + " and not supported") +) + +func uvError(read int) error { + switch { + case read == 0: + return ErrVarintBuffSmall + case read < 0: + return ErrVarintTooBig + default: + return nil + } +} + func Cast(data []byte) (*Cid, error) { if len(data) == 34 && data[0] == 18 && data[1] == 32 { h, err := mh.Cast(data) @@ -103,11 +121,18 @@ func Cast(data []byte) (*Cid, error) { } vers, n := binary.Uvarint(data) + if err := uvError(n); err != nil { + return nil, err + } + if vers != 0 && vers != 1 { return nil, fmt.Errorf("invalid cid version number: %d", vers) } codec, cn := binary.Uvarint(data[n:]) + if err := uvError(cn); err != nil { + return nil, err + } rest := data[n+cn:] h, err := mh.Cast(rest) From 5ec5bbcb48017f9f3b9440b384c4deb5ba8897de Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 17 Nov 2016 18:27:01 +0100 Subject: [PATCH 3/9] Improve cid_fuzz.go tests --- .gitignore | 1 + cid_fuzz.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..aaea8ed --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +cid-fuzz.zip diff --git a/cid_fuzz.go b/cid_fuzz.go index 495ec4d..0d47167 100644 --- a/cid_fuzz.go +++ b/cid_fuzz.go @@ -10,8 +10,26 @@ func Fuzz(data []byte) int { } _ = cid.Bytes() + _ = cid.String() + _ = cid.Prefix() + if !cid.Equals(cid) { panic("inequality") } + + // json loop + json, err := cid.MarshalJSON() + if err != nil { + panic(err.Error()) + } + cid2 := &Cid{} + err = cid2.UnmarshalJSON(json) + if err != nil { + panic(err.Error()) + } + + if !cid.Equals(cid2) { + panic("json loop not equal") + } return 1 } From 6ce8a80816b06b3f3c5e61553ee4efb1c578e4a1 Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 17 Nov 2016 18:45:34 +0100 Subject: [PATCH 4/9] Fix panic when length of varints is greater than 8 It can be 16 --- cid.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cid.go b/cid.go index 2fd43e7..3436501 100644 --- a/cid.go +++ b/cid.go @@ -187,7 +187,8 @@ func (c *Cid) bytesV0() []byte { } func (c *Cid) bytesV1() []byte { - buf := make([]byte, 8+len(c.hash)) + // two 8 bytes (max) numbers plus hash + buf := make([]byte, 2*8+len(c.hash)) n := binary.PutUvarint(buf, c.version) n += binary.PutUvarint(buf[n:], c.codec) copy(buf[n:], c.hash) From 5da6d87c58e94c007e2ba72e938e3df31a2f4586 Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 17 Nov 2016 18:50:50 +0100 Subject: [PATCH 5/9] Add test for max lenght varint --- cid_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cid_test.go b/cid_test.go index 0c51bdb..e6e479e 100644 --- a/cid_test.go +++ b/cid_test.go @@ -119,6 +119,15 @@ func TestPrefixRoundtrip(t *testing.T) { } } +func Test16BytesVarint(t *testing.T) { + data := []byte("this is some test content") + hash, _ := mh.Sum(data, mh.SHA2_256, -1) + c := NewCidV1(CBOR, hash) + + c.codec = 1 << 54 + _ = c.Bytes() +} + func TestFuzzCid(t *testing.T) { buf := make([]byte, 128) for i := 0; i < 200; i++ { From 9c3e314588419deb3eb6b8979b63821aec42f9f5 Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 17 Nov 2016 19:01:33 +0100 Subject: [PATCH 6/9] Use defined MaxVarintLen64 from stdlib --- cid.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cid.go b/cid.go index 3436501..bf553ea 100644 --- a/cid.go +++ b/cid.go @@ -188,7 +188,7 @@ func (c *Cid) bytesV0() []byte { func (c *Cid) bytesV1() []byte { // two 8 bytes (max) numbers plus hash - buf := make([]byte, 2*8+len(c.hash)) + buf := make([]byte, 2*binary.MaxVarintLen64+len(c.hash)) n := binary.PutUvarint(buf, c.version) n += binary.PutUvarint(buf[n:], c.codec) copy(buf[n:], c.hash) From c67fe910f2508f1f166d5b2452d7ce8fbc0d2b50 Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 17 Nov 2016 19:03:01 +0100 Subject: [PATCH 7/9] Test prefix.Bytes() in fuzz tests --- cid_fuzz.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cid_fuzz.go b/cid_fuzz.go index 0d47167..357e907 100644 --- a/cid_fuzz.go +++ b/cid_fuzz.go @@ -11,7 +11,8 @@ func Fuzz(data []byte) int { _ = cid.Bytes() _ = cid.String() - _ = cid.Prefix() + p := cid.Prefix() + _ = p.Bytes() if !cid.Equals(cid) { panic("inequality") @@ -31,5 +32,6 @@ func Fuzz(data []byte) int { if !cid.Equals(cid2) { panic("json loop not equal") } + return 1 } From 9116bf80253f5e5a5d2dbac8c08f202d4045cf7c Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 17 Nov 2016 19:16:05 +0100 Subject: [PATCH 8/9] Fix lengths in prefix too --- cid.go | 2 +- cid_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cid.go b/cid.go index bf553ea..04cc09a 100644 --- a/cid.go +++ b/cid.go @@ -266,7 +266,7 @@ func (p Prefix) Sum(data []byte) (*Cid, error) { } func (p Prefix) Bytes() []byte { - buf := make([]byte, 16) + buf := make([]byte, 4*binary.MaxVarintLen64) n := binary.PutUvarint(buf, p.Version) n += binary.PutUvarint(buf[n:], p.Codec) n += binary.PutUvarint(buf[n:], uint64(p.MhType)) diff --git a/cid_test.go b/cid_test.go index e6e479e..c03e281 100644 --- a/cid_test.go +++ b/cid_test.go @@ -124,7 +124,7 @@ func Test16BytesVarint(t *testing.T) { hash, _ := mh.Sum(data, mh.SHA2_256, -1) c := NewCidV1(CBOR, hash) - c.codec = 1 << 54 + c.codec = 1 << 63 _ = c.Bytes() } From ba97b640bd03ceecf2b3bb5b5ba1271e6ad0b4a3 Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 17 Nov 2016 19:24:12 +0100 Subject: [PATCH 9/9] Check length of copy to be extra sure we are copying whole thing --- cid.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cid.go b/cid.go index 04cc09a..e3aab8c 100644 --- a/cid.go +++ b/cid.go @@ -191,7 +191,10 @@ func (c *Cid) bytesV1() []byte { buf := make([]byte, 2*binary.MaxVarintLen64+len(c.hash)) n := binary.PutUvarint(buf, c.version) n += binary.PutUvarint(buf[n:], c.codec) - copy(buf[n:], c.hash) + cn := copy(buf[n:], c.hash) + if cn != len(c.hash) { + panic("copy hash length is inconsistent") + } return buf[:n+len(c.hash)] }