Skip to content

Commit

Permalink
Fix bug in merkle/iavl_proof; TODO maybe read zero length slices as nil?
Browse files Browse the repository at this point in the history
  • Loading branch information
jaekwon committed Jul 9, 2015
1 parent ff6aa08 commit d952344
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 13 deletions.
3 changes: 2 additions & 1 deletion binary/binary.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import (
// TODO document and maybe make it configurable.
const MaxBinaryReadSize = 21 * 1024 * 1024

var ErrMaxBinaryReadSizeReached = errors.New("Error: max binary read size reached")
var ErrBinaryReadSizeOverflow = errors.New("Error: binary read size overflow")
var ErrBinaryReadSizeUnderflow = errors.New("Error: binary read size underflow")

func ReadBinary(o interface{}, r io.Reader, n *int64, err *error) interface{} {
rv, rt := reflect.ValueOf(o), reflect.TypeOf(o)
Expand Down
18 changes: 14 additions & 4 deletions binary/byteslice.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package binary

import (
"io"

. "github.com/tendermint/tendermint/common"
)

func WriteByteSlice(bz []byte, w io.Writer, n *int64, err *error) {
Expand All @@ -14,8 +16,12 @@ func ReadByteSlice(r io.Reader, n *int64, err *error) []byte {
if *err != nil {
return nil
}
if MaxBinaryReadSize < *n+int64(length) {
*err = ErrMaxBinaryReadSizeReached
if length < 0 {
*err = ErrBinaryReadSizeUnderflow
return nil
}
if MaxBinaryReadSize < MaxInt64(int64(length), *n+int64(length)) {
*err = ErrBinaryReadSizeOverflow
return nil
}

Expand All @@ -41,8 +47,12 @@ func ReadByteSlices(r io.Reader, n *int64, err *error) [][]byte {
if *err != nil {
return nil
}
if MaxBinaryReadSize < *n+int64(length) {
*err = ErrMaxBinaryReadSizeReached
if length < 0 {
*err = ErrBinaryReadSizeUnderflow
return nil
}
if MaxBinaryReadSize < MaxInt64(int64(length), *n+int64(length)) {
*err = ErrBinaryReadSizeOverflow
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion binary/reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func readReflectBinary(rv reflect.Value, rt reflect.Type, opts Options, r io.Rea
return
}
if MaxBinaryReadSize < *n {
*err = ErrMaxBinaryReadSizeReached
*err = ErrBinaryReadSizeOverflow
return
}
}
Expand Down
2 changes: 1 addition & 1 deletion binary/reflect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ func TestJSONFieldNames(t *testing.T) {
func TestBadAlloc(t *testing.T) {
n, err := new(int64), new(error)
instance := new([]byte)
data := RandBytes(ByteSliceChunk * 100)
data := RandBytes(100 * 1024)
b := new(bytes.Buffer)
// this slice of data claims to be much bigger than it really is
WriteUvarint(uint(10000000000000000), b, n, err)
Expand Down
14 changes: 11 additions & 3 deletions binary/string.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package binary

import "io"
import (
"io"

. "github.com/tendermint/tendermint/common"
)

// String

Expand All @@ -14,8 +18,12 @@ func ReadString(r io.Reader, n *int64, err *error) string {
if *err != nil {
return ""
}
if MaxBinaryReadSize < *n+int64(length) {
*err = ErrMaxBinaryReadSizeReached
if length < 0 {
*err = ErrBinaryReadSizeUnderflow
return ""
}
if MaxBinaryReadSize < MaxInt64(int64(length), *n+int64(length)) {
*err = ErrBinaryReadSizeOverflow
return ""
}

Expand Down
3 changes: 1 addition & 2 deletions merkle/iavl_proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package merkle
import (
"bytes"
"crypto/sha256"

"github.com/tendermint/tendermint/binary"
. "github.com/tendermint/tendermint/common"
)
Expand Down Expand Up @@ -47,7 +46,7 @@ func (branch IAVLProofInnerNode) Hash(childHash []byte) []byte {
n, err := int64(0), error(nil)
binary.WriteInt8(branch.Height, buf, &n, &err)
binary.WriteVarint(branch.Size, buf, &n, &err)
if branch.Left == nil {
if len(branch.Left) == 0 {
binary.WriteByteSlice(childHash, buf, &n, &err)
binary.WriteByteSlice(branch.Right, buf, &n, &err)
} else {
Expand Down
1 change: 1 addition & 0 deletions merkle/iavl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ func testProof(t *testing.T, proof *IAVLProof, keyBytes, valueBytes, rootHash []
return
}
if !proof2.Verify(keyBytes, valueBytes, rootHash) {
// t.Log(Fmt("%X\n%X\n", proofBytes, binary.BinaryBytes(proof2)))
t.Errorf("Invalid proof after write/read. Verification failed.")
return
}
Expand Down
2 changes: 1 addition & 1 deletion p2p/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ func (ch *Channel) writeMsgPacketTo(w io.Writer) (n int64, err error) {
func (ch *Channel) recvMsgPacket(packet msgPacket) ([]byte, error) {
log.Debug("Read Msg Packet", "conn", ch.conn, "packet", packet)
if binary.MaxBinaryReadSize < len(ch.recving)+len(packet.Bytes) {
return nil, binary.ErrMaxBinaryReadSizeReached
return nil, binary.ErrBinaryReadSizeOverflow
}
ch.recving = append(ch.recving, packet.Bytes...)
if packet.EOF == byte(0x01) {
Expand Down
1 change: 1 addition & 0 deletions scripts/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* http://redsymbol.net/articles/unofficial-bash-strict-mode/

0 comments on commit d952344

Please sign in to comment.