Lack of protection when withdrawing Static Atoken

mediumCode4rena

Lines of code

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/aave/StaticATokenLM.sol#L324-L365

Vulnerability details

Impact

The Aave plugin is associated with an ever-increasing exchange rate. The earlier a user wraps the AToken, the more Static Atoken will be minted and understandably no slippage protection is needed.

However, since the rate is not linearly increasing, withdrawing the Static Atoken (following RToken redemption) at the wrong time could mean a difference in terms of the amount of AToken redeemed. The rate could be in a transient mode of non-increasing or barely increasing and then a significant surge. Users with an equivalent amount of Static AToken making such calls at around the same time could end up having different percentage gains.

Although the user could always deposit and wrap the AToken again, it's not going to help if the wrapping were to encounter a sudden surge (bad timing again) thereby thwarting the intended purpose.

Proof of Concept

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/aave/StaticATokenLM.sol#L338-L355

solidity📋
        uint256 userBalance = balanceOf(owner);

        uint256 amountToWithdraw;
        uint256 amountToBurn;

        uint256 currentRate = rate();
        if (staticAmount > 0) {
            amountToBurn = (staticAmount > userBalance) ? userBalance : staticAmount;
            amountToWithdraw = _staticToDynamicAmount(amountToBurn, currentRate);
        } else {
            uint256 dynamicUserBalance = _staticToDynamicAmount(userBalance, currentRate);
            amountToWithdraw = (dynamicAmount > dynamicUserBalance)
                ? dynamicUserBalance
                : dynamicAmount;
            amountToBurn = _dynamicToStaticAmount(amountToWithdraw, currentRate);
        }

        _burn(owner, amountToBurn);

As can be seen in the code block of function _withdraw above, choosing staticAmount > 0 will have a lesser amount of AToken to withdraw when the currenRate is stagnant.

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/aave/StaticATokenLM.sol#L297-L299

solidity📋
    function _staticToDynamicAmount(uint256 amount, uint256 rate_) internal pure returns (uint256) {
        return amount.rayMul(rate_);
    }

Similarly, choosing dynamicAmount > 0 will have a higher than expected amount of Static Atoken to burn.

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/aave/StaticATokenLM.sol#L293-L295

solidity📋
    function _dynamicToStaticAmount(uint256 amount, uint256 rate_) internal pure returns (uint256) {
        return amount.rayDiv(rate_);
    }

Tools Used

Manual

Recommended Mitigation Steps

Consider implementing slippage protection on StaticATokenLM._withdraw so that users could opt for the minimum amount of AToken to receive or the maximum amount of Static Atoken to burn.

Assessed type

Timing