Description

In the OnRecvPacket() function of the IBC middleware, there is a check that ensures that this packet has not already been processed by another middleware:

// if this packet has been handled by another middleware in the stack there may be no need to call into the
// underlying app, otherwise the transfer module's OnRecvPacket callback could be invoked more than once
// which would mint/burn vouchers more than once
if !processed {
	if err := im.receiveFunds(ctx, packet, data, overrideReceiver, relayer); err != nil {
		logger.Error("packetForwardMiddleware OnRecvPacket error receiving packet", "error", err)
		return newErrorAcknowledgement(fmt.Errorf("error receiving packet: %w", err))
	}
}

Currently, there are no other middlewares that can set this flag to true (it is by default set to false). Therefore, this check will always pass, and im.receiveFunds() will always be called, which will mean the overrideReceiver is always minted the vouchers that were transferred over IBC.

However, if, in the future, some middleware were to set this processed flag but not actually mint vouchers to the receiver, then it would lead to loss of funds for the user.

Impact

Currently, the severity and impact are both informational because this bug is only possible in the future, assuming that other middlewares can set this processed flag. However, the future impact and severity can be critical, because a user can lose their funds (due to no vouchers being minted).

Recommendations

Document this check extensively, preferably in the code itself as a comment. This will make sure developers are aware of this and do not introduce this issue in the future.

Remediation