Description

It was found that the Wasm bindings use messages directly after they are unmarshalled without calling ValidateBasic. The messages are directly passed to the handlers and crucial checks are skipped.

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)

Any checks that the handlers rely on ValidateBasic for are skipped and can be exploited if the respective checks are not present in the handlers.

Impact

The following are the examples of messages that can be exploited:

• For one, ExecuteMsg.AddMargin does not check if the margin denom is the same as the pair denom. This could allow incorrect collateral to be used.

• Another is that ExecuteMsg.RemoveMargin does not check that the amount to remove is positive, allowing the margin of a position to be increased without transferring any funds from the user. The inflated marging could then be withrawn to drain the VaultModuleAccount and PerpEFModuleAccount pools.

Recommendations

After creating each sdkMsg, the ValidateBasic() should be called on each before they are passed to the MsgServer in the executor.

Remediation

This issue has been acknowledged by Nibiru, and fixes were implemented in the following commits:

• ba58517e↗