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

chore(osmosis-outpost): add receiver address validation #2063

Merged
merged 6 commits into from
Nov 21, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
- (staking) [#2053](https://github.com/evmos/evmos/pull/2053) Change the validator address in the events from string type to address type.
- (werc20) [#2057](https://github.com/evmos/evmos/pull/2057) WERC20 refactors and handling of fallback and receive functions.
- (werc20) [#2062](https://github.com/evmos/evmos/pull/2062) Remove name checking for `fallback` and `receive` functions.
- (osmosis-outpost) [#202063](https://github.com/evmos/evmos/pull/2063) Check that receiver address is bech32 with "evmos" as human readable part.

### Bug Fixes

Expand Down
6 changes: 3 additions & 3 deletions precompiles/outposts/osmosis/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ package osmosis
import "fmt"

var (
// ErrEmptyReceiver is raised when the receiver used in the memo is an
// empty string.
ErrEmptyReceiver = "receiver address cannot be empty"
// ErrEmptyOnFailedDelivery is raised when the onFailedDeliver field of the
// IBC memo is an empty string.
ErrEmptyOnFailedDelivery = "onFailedDelivery cannot be empty"
Expand All @@ -25,4 +22,7 @@ var (
// ErrInputTokenNotSupported is raised when the osmosis outpost receives a non-supported
// input token for the swap.
ErrInputTokenNotSupported = "input not supported, supported tokens: %v" //#nosec G101 -- no hardcoded credentials here
// ErrReceiverAddress is raised when an error occurs during the validation of the receiver of the
// swap.
ErrReceiverAddress = "error during receiver address validation: %s"
)
24 changes: 12 additions & 12 deletions precompiles/outposts/osmosis/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (s *PrecompileTestSuite) TestSwap() {
senderAddress := utiltx.GenerateAddress()
sender := sdktypes.AccAddress(senderAddress.Bytes())
randomAddress := utiltx.GenerateAddress()
osmoAddress := "osmo1qql8ag4cluz6r4dz28p3w00dnc9w8ueuhnecd2"
receiver := "evmos1vl0x3xr0zwgrllhdzxxlkal7txnnk56q3552x7" //nolint:goconst

method := s.precompile.Methods[osmosis.SwapMethod]
testCases := []struct {
Expand Down Expand Up @@ -60,7 +60,7 @@ func (s *PrecompileTestSuite) TestSwap() {
transferAmount,
slippagePercentage,
windowSeconds,
osmoAddress,
receiver,
}
},
expError: true,
Expand All @@ -80,7 +80,7 @@ func (s *PrecompileTestSuite) TestSwap() {
transferAmount,
slippagePercentage,
windowSeconds,
osmoAddress,
receiver,
}
},
expError: true,
Expand All @@ -100,7 +100,7 @@ func (s *PrecompileTestSuite) TestSwap() {
transferAmount,
slippagePercentage,
windowSeconds,
osmoAddress,
receiver,
}
},
expError: true,
Expand All @@ -120,7 +120,7 @@ func (s *PrecompileTestSuite) TestSwap() {
transferAmount,
slippagePercentage,
windowSeconds,
osmoAddress,
receiver,
}
},
expError: true,
Expand Down Expand Up @@ -148,7 +148,7 @@ func (s *PrecompileTestSuite) TestSwap() {
transferAmount,
slippagePercentage,
windowSeconds,
osmoAddress,
receiver,
}
},
expError: true,
Expand All @@ -169,7 +169,7 @@ func (s *PrecompileTestSuite) TestSwap() {
transferAmount,
slippagePercentage,
windowSeconds,
osmoAddress,
receiver,
}
},
expError: true,
Expand All @@ -193,7 +193,7 @@ func (s *PrecompileTestSuite) TestSwap() {
transferAmount,
slippagePercentage,
windowSeconds,
osmoAddress,
receiver,
}
},
expError: true,
Expand Down Expand Up @@ -222,7 +222,7 @@ func (s *PrecompileTestSuite) TestSwap() {
}
},
expError: true,
errContains: "invalid separator",
errContains: fmt.Sprintf(osmosis.ErrReceiverAddress, "not a valid evmos address"),
}, {
// THIS PANICS INSIDE CheckAuthzExists
name: "fail - origin different from address caller",
Expand All @@ -243,7 +243,7 @@ func (s *PrecompileTestSuite) TestSwap() {
transferAmount,
slippagePercentage,
windowSeconds,
osmoAddress,
receiver,
}
},
expError: true,
Expand All @@ -267,7 +267,7 @@ func (s *PrecompileTestSuite) TestSwap() {
transferAmount,
slippagePercentage,
windowSeconds,
osmoAddress,
receiver,
}
},
expError: true,
Expand All @@ -291,7 +291,7 @@ func (s *PrecompileTestSuite) TestSwap() {
transferAmount,
slippagePercentage,
windowSeconds,
osmoAddress,
receiver,
}
},
expError: false,
Expand Down
55 changes: 26 additions & 29 deletions precompiles/outposts/osmosis/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@ import (
"math/big"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"
"github.com/evmos/evmos/v15/utils"
"golang.org/x/exp/slices"

cosmosbech32 "github.com/cosmos/cosmos-sdk/types/bech32"

"github.com/cosmos/btcutil/bech32"
"github.com/ethereum/go-ethereum/common"
cmn "github.com/evmos/evmos/v15/precompiles/common"
)
Expand Down Expand Up @@ -98,6 +96,32 @@ type Memo struct {
Msg *Msg `json:"msg"`
}

// Validate performs basic validation of the IBC memo for the Osmosis outpost.
// This function assumes that memo field is parsed with ParseSwapPacketData, which
// performs data casting ensuring outputDenom cannot be an empty string.
func (m Memo) Validate() error {
osmosisSwap := m.Msg.OsmosisSwap

if osmosisSwap.OnFailedDelivery == "" {
return fmt.Errorf(ErrEmptyOnFailedDelivery)
}

// Check if account is a valid bech32 address
if _, err := sdk.AccAddressFromBech32(osmosisSwap.Receiver); err != nil {
return fmt.Errorf(ErrReceiverAddress, "not a valid evmos address")
}

if osmosisSwap.Slippage.TWAP.SlippagePercentage == 0 || osmosisSwap.Slippage.TWAP.SlippagePercentage > MaxSlippagePercentage {
return fmt.Errorf(ErrSlippagePercentage)
}

if osmosisSwap.Slippage.TWAP.WindowSeconds == 0 || osmosisSwap.Slippage.TWAP.WindowSeconds > MaxWindowSeconds {
return fmt.Errorf(ErrWindowSeconds)
}

return nil
}

// RawPacketMetadata is the raw packet metadata used to construct a JSON string.
type RawPacketMetadata struct {
// The Osmosis outpost IBC memo.
Expand Down Expand Up @@ -145,33 +169,6 @@ func (r RawPacketMetadata) String() string {
return string(jsonBytes)
}

// Validate performs basic validation of the IBC memo for the Osmosis outpost.
// This function assumes that memo field are parsed with ParseSwapPacketData, which
// performs data casting ensuring outputDenom cannot be an empty string.
func (m Memo) Validate() error {
osmosisSwap := m.Msg.OsmosisSwap

if osmosisSwap.OnFailedDelivery == "" {
return fmt.Errorf(ErrEmptyOnFailedDelivery)
}

// Check if account is a valid bech32 address
_, _, err := bech32.Decode(osmosisSwap.Receiver, address.MaxAddrLen)
if err != nil {
return err
}

if osmosisSwap.Slippage.TWAP.SlippagePercentage == 0 || osmosisSwap.Slippage.TWAP.SlippagePercentage > MaxSlippagePercentage {
return fmt.Errorf(ErrSlippagePercentage)
}

if osmosisSwap.Slippage.TWAP.WindowSeconds == 0 || osmosisSwap.Slippage.TWAP.WindowSeconds > MaxWindowSeconds {
return fmt.Errorf(ErrWindowSeconds)
}

return nil
}

// CreateOnFailedDeliveryField is an utility function to create the memo field
// onFailedDelivery. The returned string is the bech32 of the receiver input
// or "do_nothing".
Expand Down