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

Anyone can call distributeAssets() wrecking contract state

thankyou

Summary

Anyone can call LiquidationPool.distributeAssets() and pass in any malicious asset they want. This can cause various issues with the LiquidationPool accounting.

Vulnerability Details

The LiquidationPool.distributeAssets() can be seen below:

// AUDIT: _assets is arbitrary and there are no modifiers
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) {
// AUDIT: assets are not validated to be acceptedTokens.
for (uint256 i = 0; i < _assets.length; i++) {
ILiquidationPoolManager.Asset memory asset = _assets[I];
if (asset.amount > 0) {
// AUDIT: no checks for asset.token done here. It's an arbitrary value.
(,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
uint256 _portion = asset.amount * _positionStake / stakeTotal;
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;
​
// AUDIT: no checks for asset.token done here. It's an arbitrary value
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
burnEuros += costInEuros;
if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
// AUDIT: no checks for asset.token done here. It's an arbitrary value
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);
}

This function lacks various checks:

  • No checks to see who msg.sender is

  • No validation as to whether or not the tokens are valid accepted tokens by the protocol

  • No validation as to the contents of the ILiquidationPoolManager.Asset attributes which are used in accounting updates. This includes:

    • token.clAddr

    • token.symbol

    • token.addr

Because of these lack of checks, a user can pass in an arbitrary value. For example, let's say that a user passes in an asset with the following values:

Asset {
clAddr: A chainlink address pointed to a completely different exchange rate than the expect token
symbol: A symbol that represents a different token
addr: An address controlled by the malicious user and accepts any function call
}

If this asset was passed into distributeAssets(), the following changes would occur:

  • assetPriceUsd becomes an arbitrary value as the hacker can determine which Chainlink exchange rate gives them the best odds. This influences how much staked position is decreased as well as how many EUROs are burned.

  • An arbitrary reward would be increased as the asset token symbol can point to any of the reward tokens the user can receive.

  • The token address will always succeed on safeTransferFrom or allow the hacker a re-entrancy into the contract.

Impact

The LiquidationPool contract is effectively nerfed by allowing anyone to call this function with arbitrary assets.

Tools Used

Manual Review

Recommendations

Restrict who can call this function but also what acceptable assets can be passed in. Retrieve those assets from the protocol itself and do not rely on the caller to provide those values.

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.