The Standard

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

Anybody can distribute any amount assets in any rate due to missing access control in `LiquidationPool#distributeAssets()`

Summary

distributeAssets() function in LiquidationPool is intended to execute by liquidation pool manager to distribute assets. But the distributeAssets() function lacks access control, and this lead to significant issues. The distributeAssets() function is responsible for distributing assets based on the liquidation of positions and the current exchange rates. It performs critical financial operations that should only be executed by authorized entities, such as a designated manager.

Without proper access control, any user could potentially call the distributeAssets() function. This could lead to unauthorized distribution of assets, manipulation of the liquidation process, or triggering of the function at inopportune times.

An attacker could exploit the lack of access control to distribute assets in their favor, potentially draining the contract's funds or disrupting the intended asset distribution logic.

The function's ability to determine the cost in EUROs and distribute rewards based on the liquidation of assets is a sensitive operation. If an unauthorized user can call this function, they could manipulate the liquidation logic to their advantage.

If assets are distributed incorrectly due to unauthorized access, users could lose their staked funds or not receive the assets they are entitled to, leading to financial losses.

Vulnerability Details

In the distributeAssets() function, there is currently no implemented access control mechanism to restrict unauthorized access.

File: contracts/LiquidationPool.sol
205: function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L205C1-L205C142

Within the liquidation process, distributeAssets() is invoked by LiquidationPoolManager#runLiquidation() at line number 80. The parameters passed to distributeAssets() are either precalculated or provided by the vault manager contract. However, the absence of access control in distributeAssets() means anyone can call it with their parameters, potentially exploiting the contract.

File: contracts/LiquidationPoolManager.sol
59: function runLiquidation(uint256 _tokenId) external {
60: ISmartVaultManager manager = ISmartVaultManager(smartVaultManager);
61: manager.liquidateVault(_tokenId);
.
.
65: uint256 ethBalance;
66: for (uint256 i = 0; i < tokens.length; i++) {
67: ITokenManager.Token memory token = tokens[i];
68: if (token.addr == address(0)) {
69: ethBalance = address(this).balance;
70: if (ethBalance > 0) assets[i] = ILiquidationPoolManager.Asset(token, ethBalance);
71: } else {
.
.
80: @> LiquidationPool(pool).distributeAssets{value: ethBalance}(assets, manager.collateralRate(), manager.HUNDRED_PC());

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPoolManager.sol#L59C1-L82C6

Impact

Unauthorized Execution: Without proper access control, any external actor can call the function, potentially leading to unauthorized and malicious actions.

Asset Misappropriation: An attacker could trigger the distribution of assets in a way that benefits them, possibly redirecting assets to their own account or to other accounts under their control.

Economic Damage: The contract's funds could be drained or misallocated, causing economic damage to legitimate stakeholders and undermining the contract's purpose.

Manipulation of Liquidation Outcomes: The lack of access control could allow an attacker to manipulate the outcome of liquidations, affecting the fair distribution of assets among stakeholders.

Tools Used

Manual Review

Recommendations

To address this issue, the distributeAssets() function should include an access control mechanism, such as the onlyManager modifier already defined in the contract, to restrict its execution to authorized users only.

- function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
+ function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable onlyManager {
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.