Summary
The _userState variable in the RToken contract is designed to track the index at the time of deposit or withdrawal. However, this tracking is unnecessary since the mint() and burn() functions already utilize scaled amounts. The index can be calculated directly during these operations, making the state variable redundant.
Vulnerability Details
Both function mint() / burn() uses scaled amount to be minted / burnt. Therefore, the interest can be calcualted converting scaled amount to normalized amount and there is no need to track the index.
We can calculate the current index at the timestamp of mint() / burn().
function mint(
address caller,
address onBehalfOf,
uint256 amountToMint,
uint256 index
) external override onlyReservePool returns (bool, uint256, uint256, uint256) {
if (amountToMint == 0) {
return (false, 0, 0, 0);
}
uint256 amountScaled = amountToMint.rayDiv(index);
if (amountScaled == 0) revert InvalidAmount();
uint256 scaledBalance = balanceOf(onBehalfOf);
bool isFirstMint = scaledBalance == 0;
>> uint256 balanceIncrease = 0;
>> if (_userState[onBehalfOf].index != 0 && _userState[onBehalfOf].index < index) {
>> balanceIncrease = scaledBalance.rayMul(index) - scaledBalance.rayMul(_userState[onBehalfOf].index);
>> }
>> _userState[onBehalfOf].index = index.toUint128();
_mint(onBehalfOf, amountToMint.toUint128());
emit Mint(caller, onBehalfOf, amountToMint, index);
return (isFirstMint, amountToMint, totalSupply(), amountScaled);
}
function burn(
address from,
address receiverOfUnderlying,
uint256 amount,
uint256 index
) external override onlyReservePool returns (uint256, uint256, uint256) {
if (amount == 0) {
return (0, totalSupply(), 0);
}
uint256 userBalance = balanceOf(from);
>> _userState[from].index = index.toUint128();
if(amount > userBalance){
amount = userBalance;
}
uint256 amountScaled = amount.rayMul(index);
>> _userState[from].index = index.toUint128();
_burn(from, amount.toUint128());
if (receiverOfUnderlying != address(this)) {
IERC20(_assetAddress).safeTransfer(receiverOfUnderlying, amount);
}
emit Burn(from, receiverOfUnderlying, amount, index);
return (amount, totalSupply(), amount);
}
Impact
Maintaining unnecessary state variables adds complexity to the contract and incurs unnecessary gas costs during transactions.
Tools Used
Manual Review
Recommendations
Remove _userState variable in the RToken.
function mint(
address caller,
address onBehalfOf,
uint256 amountToMint,
uint256 index
) external override onlyReservePool returns (bool, uint256, uint256, uint256) {
if (amountToMint == 0) {
return (false, 0, 0, 0);
}
uint256 amountScaled = amountToMint.rayDiv(index);
if (amountScaled == 0) revert InvalidAmount();
uint256 scaledBalance = balanceOf(onBehalfOf);
bool isFirstMint = scaledBalance == 0;
- uint256 balanceIncrease = 0;
- if (_userState[onBehalfOf].index != 0 && _userState[onBehalfOf].index < index) {
- balanceIncrease = scaledBalance.rayMul(index) - scaledBalance.rayMul(_userState[onBehalfOf].index);
- }
- _userState[onBehalfOf].index = index.toUint128();
_mint(onBehalfOf, amountToMint.toUint128());
emit Mint(caller, onBehalfOf, amountToMint, index);
return (isFirstMint, amountToMint, totalSupply(), amountScaled);
}
function burn(
address from,
address receiverOfUnderlying,
uint256 amount,
uint256 index
) external override onlyReservePool returns (uint256, uint256, uint256) {
if (amount == 0) {
return (0, totalSupply(), 0);
}
uint256 userBalance = balanceOf(from);
- _userState[from].index = index.toUint128();
if(amount > userBalance){
amount = userBalance;
}
uint256 amountScaled = amount.rayMul(index);
- _userState[from].index = index.toUint128();
_burn(from, amount.toUint128());
if (receiverOfUnderlying != address(this)) {
IERC20(_assetAddress).safeTransfer(receiverOfUnderlying, amount);
}
emit Burn(from, receiverOfUnderlying, amount, index);
return (amount, totalSupply(), amount);
}
