Skip to content

Commit

Permalink
fix vuln demonstrated by gnolang#583
Browse files Browse the repository at this point in the history
Original fix at cosmos/iavl#582, is simply:

```
if len(pin.Left) > 0 && len(pin.Right) > 0 {
	return nil, errors.New("both left and right child hashes are set")
}
```
Our iavl functions however don't return errors. Proposing to use
`panic()` instead, as it does in other parts of this file.

More about this 2022-10-07 vuln:

https://medium.com/@Beosin_com/how-did-the-bnb-chain-exploiter-pass-iavl-proof-verification-an-in-depth-analysis-by-beosin-c925b77bc13e
https://twitter.com/buchmanster/status/1578879225574350848
  • Loading branch information
grepsuzette committed Mar 10, 2023
1 parent a693e08 commit 508cd88
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkgs/iavl/proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var (

//----------------------------------------

// Contract: Left and Right can never both be set. Will result in a empty `[]` roothash
type proofInnerNode struct {
Height int8 `json:"height"`
Size int64 `json:"size"`
Expand Down Expand Up @@ -62,6 +63,10 @@ func (pin proofInnerNode) Hash(childHash []byte) []byte {
err = amino.EncodeVarint(buf, pin.Version)
}

if len(pin.Left) > 0 && len(pin.Right) > 0 {
panic(fmt.Sprintf("both left and right child hashes are set"))
}

if len(pin.Left) == 0 {
if err == nil {
err = amino.EncodeByteSlice(buf, childHash)
Expand Down

0 comments on commit 508cd88

Please sign in to comment.