The selectedAuthenticators indexes can be negative
Description
The ante handler for the Authenticator allows users to specify which Authenticator to use for any message. Every account has a registered list of authenticators that can be used. The users specify the index to be used.
selectedAuthenticators, err := ad.GetSelectedAuthenticators(extTx, len(msgs))
if err != nil {
return ctx, err
}
// Authenticate the accounts of all messages
for msgIndex, msg := range msgs {
[...]
var authenticators []types.Authenticator
if selectedAuthenticators[msgIndex] == -1 {
authenticators = allAuthenticators
} else {
if int(selectedAuthenticators[msgIndex]) >= len(allAuthenticators) {
return ctx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, fmt.Sprintf("invalid authenticator index for message %d", msgIndex))
}
authenticators = []types.Authenticator{allAuthenticators[selectedAuthenticators[msgIndex]]}
}The ante handler checks if the index is greater than the length of all registered authenticators. However, this condition will still be true if msgIndex is negative.
Impact
The ante handler will panic if a negative index is used for the authenticator. However, this panic is handled by the caller and the transaction is aborted.
func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, result *sdk.Result, anteEvents []abci.Event, err error) {
[...]
defer func() {
if r := recover(); r != nil {
recoveryMW := newOutOfGasRecoveryMiddleware(gasWanted, ctx, app.runTxRecoveryMiddleware)
err, result = processRecovery(r, recoveryMW), nil
ctx.Logger().Error("panic recovered in runTx", "err", err)
}
gInfo = sdk.GasInfo{GasWanted: gasWanted, GasUsed: ctx.GasMeter().GasConsumed()}
}()Recommendations
The ante handler should check if the index for the selected authenticator is negative.
Remediation
This issue has been acknowledged by Osmosis Labs, and a fix was implemented in commit 1e2b57a6↗. Negative indices are now disallowed.