The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Native tokens can be stolen by manipulating rewards in distributeAssets()

georgishishkov

Summary

A malicios user can call distributeAssets() with carefully selected input, backrunning a transaction that liquidates native tokens, and steal all native token rewards. The input would consist of the _assets array only having a native token entry with address(0), a large amount and high _collateralRate and _hundredPC, causing rewards to be added with little cost to the user position. The malicios user will then call claimRewards() and steal the native token rewards from the LiquidationPool and thus other stakers.

Vulnerability Details

The distributeAssets() function does not have any validation on msg.sender, meaning anybody can call it. Normally this function would only be called by LiquidationPoolManager in runLiquidation()(see code below) during vault liquidations and would revert if any ERC20s are passed during the safeTransferFrom() in distributeAssets()(see code below) call, pulling the ERC20 funds from the LiquidationPoolManager:

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L229

if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
// assets pulled from manager to this contract
// @audit this would revert if any ERC20 is passed in the _assets array,
// however it will not revert if the only passed token is
// the native token as this line will not be executed.
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}

However, there are no checks that make the transaction revert if the input array only cosists of the native token.
This is a consequence of the design of asset transfer within the protocol. When ERC20s are liquidated they are first approved by the LiquidationPoolManager and then pulled by the LiquidationPool making the transaction revert if there is no approval, however when native tokens are liquidated, they are directly sent to the LiquidationPool, without any checks on the LiquidationPool side.

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPoolManager.sol#L59

runLiquidation() Code
function runLiquidation(uint256 _tokenId) external {
ISmartVaultManager manager = ISmartVaultManager(smartVaultManager);
manager.liquidateVault(_tokenId);
distributeFees();
ITokenManager.Token[] memory tokens = ITokenManager(manager.tokenManager()).getAcceptedTokens();
ILiquidationPoolManager.Asset[] memory assets = new ILiquidationPoolManager.Asset[](tokens.length);
uint256 ethBalance;
for (uint256 i = 0; i < tokens.length; i++) {
ITokenManager.Token memory token = tokens[i];
if (token.addr == address(0)) {
ethBalance = address(this).balance;
if (ethBalance > 0) assets[i] = ILiquidationPoolManager.Asset(token, ethBalance);
} else {
IERC20 ierc20 = IERC20(token.addr);
uint256 erc20balance = ierc20.balanceOf(address(this));
if (erc20balance > 0) {
assets[i] = ILiquidationPoolManager.Asset(token, erc20balance);
// @audit ERC20 tokens approved for LiquidationPool to pull
ierc20.approve(pool, erc20balance);
}
}
}
// @audit native tokens, e.g. ethBalance, directly sent to LiquidationPool
LiquidationPool(pool).distributeAssets{value: ethBalance}(assets, manager.collateralRate(), manager.HUNDRED_PC());
forwardRemainingRewards(tokens);
}

Let's now take a more detailed look at how amount and costInEuros can be manipulated inside distributeAssets()(see code below).
The _assets array is composed of ILiquidationPoolManager.Asset and ITokenManager.Token, which look like this:

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/interfaces/ILiquidationPoolManager.sol#L7

struct Asset { ITokenManager.Token token; uint256 amount; }

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/interfaces/ITokenManager.sol#L5

struct Token { bytes32 symbol; address addr; uint8 dec; address clAddr; uint8 clDec; }

A malicios user can select amount such that it is large enough that his _portion covers all native token rewards in the LiquidationPool.
In distributeAssets() we can see that _portion is calculated as follows:

uint256 _portion = asset.amount * _positionStake / stakeTotal;

Suppose there is 100 liquidated ETH available in the LiquidationPool for claiming and the malicios user has 1% of the total stake.
After a normal transaction, the user would have 1% of that ETH, so 1 ETH available in rewards to claim. The malicios user can call the function again with amount= 100*100 = 10000 and have _portion=100, making 100 ETH available as rewards for himself.
Under normal circuimstances, these are 'rewards' subtracted from the current EUROs stake of the user, however this can too be manipulated with the input in four ways:

  • very high _collateralrate

  • very high _hundredPC

  • higher token decimals(for ERC20s with less than 18)

  • oracle address returning lower price

In distributeAssets() costInEuros is calculated as follows:

uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd)
/ uint256(priceEurUsd) * _hundredPC / _collateralRate;

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L205

distributeAssets() Code
function distributeAssets(
ILiquidationPoolManager.Asset[] memory _assets,
uint256 _collateralRate,
uint256 _hundredPC
) external payable {
consolidatePendingStakes();
(, int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
uint256 stakeTotal = getStakeTotal();
uint256 burnEuros;
uint256 nativePurchased;
for (uint256 j = 0; j < holders.length; j++) {
Position memory _position = positions[holders[j]];
uint256 _positionStake = stake(_position);
if (_positionStake > 0) {
for (uint256 i = 0; i < _assets.length; i++) {
ILiquidationPoolManager.Asset memory asset = _assets[i];
if (asset.amount > 0) {
// @audit malicios user can also manipulate asset.token.clAddr, to give a lower
// value than expected, driving down the costInEuros
(, int256 assetPriceUsd,,,) =
Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
// calculate amount as stake percentage
uint256 _portion = asset.amount * _positionStake / stakeTotal;
// @audit malicios user would drive down the cost in euros by selecting
// very high _hundredPC and _collateralRate.
uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd)
/ uint256(priceEurUsd) * _hundredPC / _collateralRate;
if (costInEuros > _position.EUROs) {
_portion = _portion * _position.EUROs / costInEuros;
costInEuros = _position.EUROs;
}
_position.EUROs -= costInEuros;
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
burnEuros += costInEuros;
if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}
}
}
}
positions[holders[j]] = _position;
}
if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
returnUnpurchasedNative(_assets, nativePurchased);
}

As we can see rewards are increased by _portion. They can be later claimed in the claimRewards() function.

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L164

claimRewards() Code
function claimRewards() external {
ITokenManager.Token[] memory _tokens = ITokenManager(tokenManager).getAcceptedTokens();
for (uint256 i = 0; i < _tokens.length; i++) {
ITokenManager.Token memory _token = _tokens[i];
uint256 _rewardAmount = rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_rewardAmount > 0) {
delete rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_token.addr == address(0)) {
(bool _sent,) = payable(msg.sender).call{value: _rewardAmount}("");
require(_sent);
} else {
IERC20(_token.addr).transfer(msg.sender, _rewardAmount);
}
}
}
}

After the malicios user completes the transaction having increased their rewards by a large amount, and having close to 0, or even 0 costInEuros, they can just call claimRewards() and steal native tokens from the LiquidationPool and thus other stakers.

The malicios user is only required to have a small stake in the LiquidationPool before the transaction.
A side effect of this attack would be all stakers having inflated native token rewards.

This attack vector can be further used to DoS the distributeAssets() function by bringing the highest stakers rewards close to the maximum uint256, causing the transactions to revert on overflow on any attempt to liquidate a vault that has native tokens because their rewards would not be able to be increased any further.

Impact

Native tokens can be stolen from the vault and all stakers will have permanently inflated native token rewards.

Tools Used

Manual Review

Recommendations

The dev team can handle this attack vector by for example requiring distributeAssets() to only be called by the intended actors. If the function is only inteded to be called by the LiquidationPoolManager, add the onlyManager modifier. This will prevent the function from being called by any random users with manipulated input.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

distributeAssets-issue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.