The issueTo() function lacks the slippage control that allows the users to revert if the amount of the underlying tokens they deposits is bigger than the amount they expected. Therefore, Assets for the affected users may be lose.
Proof of Concept
During issuance, the user deposits the underlying tokens into the RToken contract, and the RToken contract uses the issueTo() function to issue RToken to the user.
function issueTo(address recipient, uint256 amount) public notIssuancePausedOrFrozen {
require(amount != 0, "Cannot issue zero");
// == Refresh ==
110: assetRegistry.refresh();
...
uint256 supply = totalSupply();
SNIP...
133: uint192 amtBaskets = supply != 0
? basketsNeeded.muluDivu(amount, supply, CEIL)
: _safeWrap(amount);
emit Issuance(issuer, recipient, amount, amtBaskets);
// Get quote from BasketHandler including issuance premium
139: (address[] memory erc20s, uint256[] memory deposits) = basketHandler.quote(
amtBaskets,
true,
CEIL
);
// == Interactions: Create RToken + transfer tokens to BackingManager ==
_scaleUp(recipient, amtBaskets, supply);
for (uint256 i = 0; i < erc20s.length; ++i) {
IERC20Upgradeable(erc20s[i]).safeTransferFrom(
issuer,
address(backingManager),
deposits[i]
);
}
}
As you can see, slippage control is not implemented in the issueTo() function.
However, slippage can occur in the issueTo() function due to various reasons.
Calculation of the BU change based on totalSupply() and basketsNeeded
However, totalSupply() and basketsNeeded can increase or decrease at any time depending on the current on-chain conditions when the transaction is executed.
_scaleUp
function _scaleUp(
address recipient,
uint192 amtBaskets,
uint256 totalSupply
) private {
// take advantage of 18 decimals during casting
uint256 amtRToken = totalSupply != 0
? amtBaskets.muluDivu(totalSupply, basketsNeeded) // {rTok} = {BU} * {qRTok} * {qRTok}
: amtBaskets; // {rTok}
emit BasketsNeededChanged(basketsNeeded, basketsNeeded + amtBaskets);
basketsNeeded += amtBaskets;
// Mint RToken to recipient
_mint(recipient, amtRToken);
}
coll.refPerTok() It depends on the token, but it can also increase or decrease at any time depending on the current on-chain conditions when the transaction is executed.
In summary, the issueTo() function lacks the slippage control that allows the users to revert if the amount of the underlying tokens they deposits is bigger than the amount they expected.
To prevent this, the redeemCustom() function implements slippage control as follows.
function redeemCustom(
...
) external notFrozen {
SNIP...
// === Save initial recipient balances ===
uint256[] memory pastBals = new uint256[](expectedERC20sOut.length);
for (uint256 i = 0; i < expectedERC20sOut.length; ++i) {
pastBals[i] = IERC20(expectedERC20sOut[i]).balanceOf(recipient);
// we haven't verified this ERC20 is registered but this is always a staticcall
}
SNIP...
// === Post-checks ===
// Check post-balances
for (uint256 i = 0; i < expectedERC20sOut.length; ++i) {
uint256 bal = IERC20(expectedERC20sOut[i]).balanceOf(recipient);
// we haven't verified this ERC20 is registered but this is always a staticcall
require(bal - pastBals[i] >= minAmounts[i], "redemption below minimum");
}
}
Considering this, I think this issue could be a valid medium.
Tools Used
Manual Review
Recommended Mitigation Steps
Implement a slippage control that allows the users to revert if the amount of the underlying tokens they deposits is bigger than the amount they expected.
Lines of code
https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/RToken.sol#L105-L155
Vulnerability details
Impact
The
issueTo()
function lacks the slippage control that allows the users to revert if the amount of the underlying tokens they deposits is bigger than the amount they expected. Therefore, Assets for the affected users may be lose.Proof of Concept
During issuance, the user deposits the underlying tokens into the
RToken
contract, and theRToken
contract uses theissueTo()
function to issue RToken to the user.As you can see, slippage control is not implemented in the
issueTo()
function.However, slippage can occur in the
issueTo()
function due to various reasons.Calculation of the BU change based on
totalSupply()
andbasketsNeeded
The BU change is calculated as follows:
However,
totalSupply()
andbasketsNeeded
can increase or decrease at any time depending on the current on-chain conditions when the transaction is executed._scaleUp
_scaleDown
Calculating
deposits[]
based oncoll.refPerTok()
The
issueTo()
function calculatesdeposits[]
using thebasketHandler.quote()
function based on the calculated BU change in #L139.The
quote()
function determinesdeposits[]
based oncoll.refPerTok()
.coll.refPerTok()
It depends on the token, but it can also increase or decrease at any time depending on the current on-chain conditions when the transaction is executed.In summary, the
issueTo()
function lacks the slippage control that allows the users to revert if the amount of the underlying tokens they deposits is bigger than the amount they expected.To prevent this, the
redeemCustom()
function implements slippage control as follows.Considering this, I think this issue could be a valid medium.
Tools Used
Manual Review
Recommended Mitigation Steps
Implement a slippage control that allows the users to revert if the amount of the underlying tokens they deposits is bigger than the amount they expected.
Assessed type
MEV