The sender is not checked for Wasm messages
Description
The CosmosWasm module has been enabled to allow developers to deploy smart contracts on Nibiru. To allow these contracts to interact with the chain, a custom executor has been written that will intercept and execute the appropriate custom calls:
type OpenPosition struct {
Sender string `json:"sender"`
Pair string `json:"pair"`
IsLong bool `json:"is_long"`
QuoteAmount sdk.Int `json:"quote_amount"`
Leverage sdk.Dec `json:"leverage"`
BaseAmountLimit sdk.Int `json:"base_amount_limit"`
}
// DispatchMsg encodes the wasmVM message and dispatches it.
func (messenger *CustomWasmExecutor) DispatchMsg(
ctx sdk.Context,
contractAddr sdk.AccAddress,
contractIBCPortID string,
wasmMsg wasmvmtypes.CosmosMsg,
) (events []sdk.Event, data [][]byte, err error) {
// If the "Custom" field is set, we handle a BindingMsg.
if wasmMsg.Custom != nil {
var contractExecuteMsg BindingExecuteMsgWrapper
if err := json.Unmarshal(wasmMsg.Custom, &contractExecuteMsg); err != nil {
return events, data, sdkerrors.Wrapf(err, "wasmMsg: %s", wasmMsg.Custom)
}
switch {
// Perp module
case contractExecuteMsg.ExecuteMsg.OpenPosition != nil:
cwMsg := contractExecuteMsg.ExecuteMsg.OpenPosition
_, err = messenger.Perp.OpenPosition(cwMsg, ctx)
return events, data, err
...These can then be called from a Cosmos contract:
/// NibiruExecuteMsg is an override of CosmosMsg::Custom. Using this msg
/// wrapper for the ExecuteMsg handlers show that their return values are valid
/// instances of CosmosMsg::Custom in a type-safe manner. It also shows how
/// ExecuteMsg can be extended in the contract.
#[cw_serde]
#[cw_custom]
pub struct NibiruExecuteMsg {
pub route: NibiruRoute,
pub msg: ExecuteMsg,
}
pub fn open_position(
sender: String,
pair: String,
is_long: bool,
quote_amount: Uint128,
leverage: Decimal,
base_amount_limit: Uint128,
) -> CosmosMsg<NibiruExecuteMsg> {
NibiruExecuteMsg {
route: NibiruRoute::Perp,
msg: ExecuteMsg::OpenPosition {
sender,
pair,
is_long,
quote_amount,
leverage,
base_amount_limit,
},
}
.into()
}The issue is that there is no validation on the value of sender; it can be set to an arbitrary account and end up being sent straight to the message handler:
func (exec *ExecutorPerp) OpenPosition(
cwMsg *cw_struct.OpenPosition, ctx sdk.Context,
) (
sdkResp *perpv2types.MsgOpenPositionResponse, err error,
) {
if cwMsg == nil {
return sdkResp, wasmvmtypes.InvalidRequest{Err: "null open position msg"}
}
pair, err := asset.TryNewPair(cwMsg.Pair)
if err != nil {
return sdkResp, err
}
var side perpv2types.Direction
if cwMsg.IsLong {
side = perpv2types.Direction_LONG
} else {
side = perpv2types.Direction_SHORT
}
sdkMsg := &perpv2types.MsgOpenPosition{
Sender: cwMsg.Sender,
Pair: pair,
Side: side,
QuoteAssetAmount: cwMsg.QuoteAmount,
Leverage: cwMsg.Leverage,
BaseAssetAmountLimit: cwMsg.BaseAmountLimit,
}
goCtx := sdk.WrapSDKContext(ctx)
return exec.MsgServer().OpenPosition(goCtx, sdkMsg)
}Impact
This allows a CosmosWasm contract to execute the OpenPosition, ClosePosition, AddMargin, and RemoveMargin operations on behalf of any user.
Recommendations
The sender should not be able to be arbitrarily set; it should be the address of the contract that is executing the message. If the sender needs to be configurable, only a whitelisted or trusted contract should be able to do it and that contract should have the appropriate checks to ensure the sender is set to the correct value.
Remediation
This issue has been acknowledged by Nibiru, and fixes were implemented in the following commits: