Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: enforce minimal encoding #99

Merged
merged 1 commit into from
Feb 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 30 additions & 42 deletions cid.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,29 +22,20 @@ package cid
import (
"bytes"
"encoding"
"encoding/binary"
"encoding/json"
"errors"
"fmt"
"strings"

mbase "github.com/multiformats/go-multibase"
mh "github.com/multiformats/go-multihash"
varint "github.com/multiformats/go-varint"
)

// UnsupportedVersionString just holds an error message
const UnsupportedVersionString = "<unsupported cid version>"

var (
// ErrVarintBuffSmall means that a buffer passed to the cid parser was not
// long enough, or did not contain an invalid cid
ErrVarintBuffSmall = errors.New("reading varint: buffer too small")

// ErrVarintTooBig means that the varint in the given cid was above the
// limit of 2^64
ErrVarintTooBig = errors.New("reading varint: varint bigger than 64bits" +
" and not supported")

// ErrCidTooShort means that the cid passed to decode was not long
// enough to be a valid Cid
ErrCidTooShort = errors.New("cid too short")
Expand Down Expand Up @@ -173,9 +164,9 @@ func NewCidV0(mhash mh.Multihash) Cid {
func NewCidV1(codecType uint64, mhash mh.Multihash) Cid {
hashlen := len(mhash)
// two 8 bytes (max) numbers plus hash
buf := make([]byte, 2*binary.MaxVarintLen64+hashlen)
n := binary.PutUvarint(buf, 1)
n += binary.PutUvarint(buf[n:], codecType)
buf := make([]byte, 1+varint.UvarintSize(codecType)+hashlen)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to say, "let's just keep it a static allocation as it gives compiler more freedom" but it is already +hashlen so it doesn't matter.

n := varint.PutUvarint(buf, 1)
n += varint.PutUvarint(buf[n:], codecType)
cn := copy(buf[n:], mhash)
if cn != hashlen {
panic("copy hash length is inconsistent")
Expand Down Expand Up @@ -281,17 +272,6 @@ func ExtractEncoding(v string) (mbase.Encoding, error) {
return encoding, nil
}

func uvError(read int) error {
switch {
case read == 0:
return ErrVarintBuffSmall
case read < 0:
return ErrVarintTooBig
default:
return nil
}
}

// Cast takes a Cid data slice, parses it and returns a Cid.
// For CidV1, the data buffer is in the form:
//
Expand Down Expand Up @@ -351,8 +331,8 @@ func (c Cid) Type() uint64 {
if c.Version() == 0 {
return DagProtobuf
}
_, n := uvarint(c.str)
codec, _ := uvarint(c.str[n:])
_, n, _ := uvarint(c.str)
codec, _, _ := uvarint(c.str[n:])
return codec
}

Expand Down Expand Up @@ -414,9 +394,9 @@ func (c Cid) Hash() mh.Multihash {
}

// skip version length
_, n1 := binary.Uvarint(bytes)
_, n1, _ := varint.FromUvarint(bytes)
// skip codec length
_, n2 := binary.Uvarint(bytes[n1:])
_, n2, _ := varint.FromUvarint(bytes[n1:])

return mh.Multihash(bytes[n1+n2:])
}
Expand Down Expand Up @@ -562,34 +542,42 @@ func (p Prefix) Sum(data []byte) (Cid, error) {
//
// <version><codec><mh-type><mh-length>
func (p Prefix) Bytes() []byte {
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))
n += binary.PutUvarint(buf[n:], uint64(p.MhLength))
return buf[:n]
size := varint.UvarintSize(p.Version)
size += varint.UvarintSize(p.Codec)
size += varint.UvarintSize(p.MhType)
size += varint.UvarintSize(uint64(p.MhLength))

buf := make([]byte, size)
n := varint.PutUvarint(buf, p.Version)
n += varint.PutUvarint(buf[n:], p.Codec)
n += varint.PutUvarint(buf[n:], p.MhType)
n += varint.PutUvarint(buf[n:], uint64(p.MhLength))
if n != size {
panic("size mismatch")
}
return buf
}

// PrefixFromBytes parses a Prefix-byte representation onto a
// Prefix.
func PrefixFromBytes(buf []byte) (Prefix, error) {
r := bytes.NewReader(buf)
vers, err := binary.ReadUvarint(r)
vers, err := varint.ReadUvarint(r)
if err != nil {
return Prefix{}, err
}

codec, err := binary.ReadUvarint(r)
codec, err := varint.ReadUvarint(r)
if err != nil {
return Prefix{}, err
}

mhtype, err := binary.ReadUvarint(r)
mhtype, err := varint.ReadUvarint(r)
if err != nil {
return Prefix{}, err
}

mhlen, err := binary.ReadUvarint(r)
mhlen, err := varint.ReadUvarint(r)
if err != nil {
return Prefix{}, err
}
Expand All @@ -616,17 +604,17 @@ func CidFromBytes(data []byte) (int, Cid, error) {
return 34, Cid{string(h)}, nil
}

vers, n := binary.Uvarint(data)
if err := uvError(n); err != nil {
vers, n, err := varint.FromUvarint(data)
if err != nil {
return 0, Undef, err
}

if vers != 1 {
return 0, Undef, fmt.Errorf("expected 1 as the cid version number, got: %d", vers)
}

_, cn := binary.Uvarint(data[n:])
if err := uvError(cn); err != nil {
_, cn, err := varint.FromUvarint(data[n:])
if err != nil {
return 0, Undef, err
}

Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ module github.com/ipfs/go-cid

require (
github.com/multiformats/go-multibase v0.0.1
github.com/multiformats/go-multihash v0.0.10
github.com/multiformats/go-multihash v0.0.13
github.com/multiformats/go-varint v0.0.5
)

go 1.13
18 changes: 10 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,20 @@ github.com/minio/sha256-simd v0.1.1-0.20190913151208-6de447530771 h1:MHkK1uRtFbV
github.com/minio/sha256-simd v0.1.1-0.20190913151208-6de447530771/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=
github.com/mr-tron/base58 v1.1.0 h1:Y51FGVJ91WBqCEabAi5OPUz38eAx8DakuAm5svLcsfQ=
github.com/mr-tron/base58 v1.1.0/go.mod h1:xcD2VGqlgYjBdcBLw+TuYLr8afG+Hj8g2eTVqeSzSU8=
github.com/mr-tron/base58 v1.1.2 h1:ZEw4I2EgPKDJ2iEw0cNmLB3ROrEmkOtXIkaG7wZg+78=
github.com/mr-tron/base58 v1.1.2/go.mod h1:BinMc/sQntlIE1frQmRFPUoPA1Zkr8VRgBdjWI2mNwc=
github.com/mr-tron/base58 v1.1.3 h1:v+sk57XuaCKGXpWtVBX8YJzO7hMGx4Aajh4TQbdEFdc=
github.com/mr-tron/base58 v1.1.3/go.mod h1:BinMc/sQntlIE1frQmRFPUoPA1Zkr8VRgBdjWI2mNwc=
github.com/multiformats/go-base32 v0.0.3 h1:tw5+NhuwaOjJCC5Pp82QuXbrmLzWg7uxlMFp8Nq/kkI=
github.com/multiformats/go-base32 v0.0.3/go.mod h1:pLiuGC8y0QR3Ue4Zug5UzK9LjgbkL8NSQj0zQ5Nz/AA=
github.com/multiformats/go-multibase v0.0.1 h1:PN9/v21eLywrFWdFNsFKaU04kLJzuYzmrJR+ubhT9qA=
github.com/multiformats/go-multibase v0.0.1/go.mod h1:bja2MqRZ3ggyXtZSEDKpl0uO/gviWFaSteVbWT51qgs=
github.com/multiformats/go-multihash v0.0.8 h1:wrYcW5yxSi3dU07n5jnuS5PrNwyHy0zRHGVoUugWvXg=
github.com/multiformats/go-multihash v0.0.8/go.mod h1:YSLudS+Pi8NHE7o6tb3D8vrpKa63epEDmG8nTduyAew=
github.com/multiformats/go-multihash v0.0.9 h1:aoijQXYYl7Xtb2pUUP68R+ys1TlnlR3eX6wmozr0Hp4=
github.com/multiformats/go-multihash v0.0.9/go.mod h1:YSLudS+Pi8NHE7o6tb3D8vrpKa63epEDmG8nTduyAew=
github.com/multiformats/go-multihash v0.0.10 h1:lMoNbh2Ssd9PUF74Nz008KGzGPlfeV6wH3rit5IIGCM=
github.com/multiformats/go-multihash v0.0.10/go.mod h1:YSLudS+Pi8NHE7o6tb3D8vrpKa63epEDmG8nTduyAew=
github.com/multiformats/go-multihash v0.0.12 h1:i2PQZWlsq8asUZwPS5w7VMCoUKEz/k/XG6WH30gsTU8=
github.com/multiformats/go-multihash v0.0.12/go.mod h1:2+oRiLVLqXx+yxM/RIdhLstp1q2WMJAlQ4kdLkS3un0=
github.com/multiformats/go-multihash v0.0.13 h1:06x+mk/zj1FoMsgNejLpy6QTvJqlSt/BhLEy87zidlc=
github.com/multiformats/go-multihash v0.0.13/go.mod h1:VdAWLKTwram9oKAatUcLxBNUjdtcVwxObEQBtRfuyjc=
github.com/multiformats/go-varint v0.0.4 h1:CplQWhUouUgTZ53vNFE8VoWr2VjaKXci+xyrKyyFuSw=
github.com/multiformats/go-varint v0.0.4/go.mod h1:3Ls8CIEsrijN6+B7PbrXRPxHRPuXSrVKRY101jdMZYE=
github.com/multiformats/go-varint v0.0.5 h1:XVZwSo04Cs3j/jS0uAEPpT3JY6DzMcVLLoWOSnCxOjg=
github.com/multiformats/go-varint v0.0.5/go.mod h1:3Ls8CIEsrijN6+B7PbrXRPxHRPuXSrVKRY101jdMZYE=
github.com/spaolacci/murmur3 v1.1.0 h1:7c1g84S4BPRrfL5Xrdp6fOJ206sU9y293DDHaoy0bLI=
github.com/spaolacci/murmur3 v1.1.0/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
Expand Down
14 changes: 10 additions & 4 deletions varint.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package cid

import (
"github.com/multiformats/go-varint"
)

// Version of varint function that work with a string rather than
// []byte to avoid unnecessary allocation

Expand All @@ -15,20 +19,22 @@ package cid
// n < 0: value larger than 64 bits (overflow)
// and -n is the number of bytes read
//
func uvarint(buf string) (uint64, int) {
func uvarint(buf string) (uint64, int, error) {
var x uint64
var s uint
// we have a binary string so we can't use a range loope
for i := 0; i < len(buf); i++ {
b := buf[i]
if b < 0x80 {
if i > 9 || i == 9 && b > 1 {
return 0, -(i + 1) // overflow
return 0, 0, varint.ErrOverflow
} else if b == 0 && i > 0 {
return 0, 0, varint.ErrNotMinimal
}
return x | uint64(b)<<s, i + 1
return x | uint64(b)<<s, i + 1, nil
}
x |= uint64(b&0x7f) << s
s += 7
}
return 0, 0
return 0, 0, varint.ErrUnderflow
}
16 changes: 12 additions & 4 deletions varint_test.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,25 @@
package cid

import (
"encoding/binary"
"testing"

"github.com/multiformats/go-varint"
)

func TestUvarintRoundTrip(t *testing.T) {
testCases := []uint64{0, 1, 2, 127, 128, 129, 255, 256, 257, 1<<63 - 1}
for _, tc := range testCases {
t.Log("testing", tc)
buf := make([]byte, 16)
binary.PutUvarint(buf, tc)
v, l1 := uvarint(string(buf))
_, l2 := binary.Uvarint(buf)
varint.PutUvarint(buf, tc)
v, l1, err := uvarint(string(buf))
if err != nil {
t.Fatalf("%v: %s", buf, err)
}
_, l2, err := varint.FromUvarint(buf)
if err != nil {
t.Fatal(err)
}
if tc != v {
t.Errorf("roundtrip failed expected %d but got %d", tc, v)
}
Expand Down