The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Incorrect Accounting in `LiquidationPool::distributeAssets()` Causes Permanent Loss of Holders' EUROs Position

krisrenzo

Description:

The formula for calculating costInEuros in the distributeAsset() function is incorrect. It erroneously multiplies _portion by 10e(18 - asset.token.dec) instead of the correct 1e(18 - asset.token.dec), resulting in a cost that is an order of magnitude higher, causing the following if block to run and make erroneous changes to stakers position.


Impact:
This error wipes out the EUROs position of holders without compensating them with the corresponding reward.


Proof of Concept:

Vulnerability Breakdown in Pseudocode

Given the following scenario:

  • Liquidated Asset: wBtc = 0.005e8 (one token for simplicity)

  • btcusd price = 40000e8

  • wbtc decimals = 8

  • eurusd price = 1.12e8

  • _hundredPC = 1e5

  • _collateralRate = 110000

Position0.EUROs value = 100e18 * 1e0 * 1.12e8 * 1e-8 = 112e8
Position1.EUROs value = 0 * 1e0 * 1.12e8 * 1e-8 = 0
Position2.EUROs value = 100e18 * 1e0 * 1.12e8 * 1e-8 = 112e8
Position3.EUROs value = 5e18 * 1e0 * 1.12e8 * 1e-8 = 5.6e8
Position4.EUROs value = 100e18 * 1e0 * 1.12e8 * 1e-8 = 112e8

Total Euros Locked = 341.6e8

Note:
- Focus is on EUROs as its the only token in the pool that contributes to the objective function of the pool

distributeAssets() logic breakdown

stakeTotal = getStakeTotal() == 341.6e8

Note:
The bugs in getStakeTotal() caused by incorrect comparison of TST and EUROs, and inaccurately calculation of total stake is assume to have been fixed, in effort to show differences in root causes. However, the Proof of Code, shows this bug persist with the original contract state.

burnEuros == 0; // tracks EUROs used up by the pool, to be burnt for supply balance
nativePurchased == 0; // tracks native token bought by the pool -- not relevant here

For Loop: loop through all holders and distribute rewards to them
{
Loop 1 //for position 0
_position = Position0
_positionStake = stake(_position) == 112e8

(if 112e8 > 0) {
Loop 1 (for wbtc)
asset = wbtc

(if 0.005e8 > 0) {
_portion = 0.005e8 * 112e8 / 341.6e8 == 0.00163934e8
costInEuros = 0.00163934e8 * 10e10 * 40000e8 / 1.12e8 * 1e5 / 110000 == 532.25325e18 // Should be 53.25e18

(if 532.25455e18 > 100e18) { // This condition should have been false
_portion = 0.00163934e8 * 100e18 / 532.25455e18 == 0.00030799.924585e8 // 0
costInEuros = _position.EUROs == 100e8 ;
}

Position0.EUROs -= 100e8 // wipes out stakers position
burnEuros += 100e8
(if reward native is token) {
// We are not dealing with a native token

}
(if reward native is erc20) {
// transfers 0 tokens to the pool.
}
}
}
// Update Position0
}
// After looping through all holders it:
// burns the burnEuros
// Return native tokens that weren't bought

Key:
{logic} : Logic block
bold font : code variables
(conditional statements)

Proof of Code:

Code

The provided test suite demonstrates the vulnerability's validity and severity.

How to Run the Test:

  • Due to the file size required to run this PoC, the suite is hosted on Github.

  • To run the PoC, clone the repository.

  • Minor changes, such as modifying function visibility, were made to enable successful test runs.

  • All changes and additional files made to the original code are documented in the README and the respective files where the changes are made.

Requirements:

  • Install Foundry.

  • Clone the project codebase into your local workspace.

    git clone https://github.com/Renzo1/the-standard-protocol-2.git

  • Run the following commands to install dependencies:

    npm install
    forge install

  • Run the following command to execute the PoC:

    forge test --match-test "testAccountErrorLoss10e" -vvv
function testAccountErrorLoss10e() public {
​
ISmartVault[] memory vaults = new ISmartVault[](1);
vaults = createVaultOwners(1);
​
//////// Owner 1 variables ////////
ISmartVault vault1 = vaults[0];
address owner1 = vault1.owner();
uint256 tstBalance1 = TST.balanceOf(owner1);
uint256 euroBalance1 = EUROs.balanceOf(owner1);
​
// Assert owner has EUROs and TST
// i.e., mint vault.mint() in TSBuilder::createVaultOwners is activated
assertGt(tstBalance1, 45 * 1e18);
assertGt(euroBalance1, 45_000 * 1e18);
​
//////// Create two random accounts Transfer tokens to them ////////
address account1 = vm.addr(111222);
address account2 = vm.addr(888999);
​
vm.startPrank(owner1);
TST.transfer(account1, 20 * 1e18);
TST.transfer(account2, 20 * 1e18);
EUROs.transfer(account1, 20_000 * 1e18);
EUROs.transfer(account2, 20_000 * 1e18);
vm.stopPrank();
​
uint256 account1TstBalance = TST.balanceOf(account1);
uint256 account2TstBalance = TST.balanceOf(account2);
uint256 account1EurosBalance = EUROs.balanceOf(account1);
uint256 account2EurosBalance = EUROs.balanceOf(account2);
​
assertEq(account1TstBalance, 20 * 1e18, "TEST 1");
assertEq(account2TstBalance, 20 * 1e18, "TEST 2");
assertEq(account1EurosBalance, 20_000 * 1e18, "TEST 3");
assertEq(account2EurosBalance, 20_000 * 1e18, "TEST 4");
​
//////// Stake Tokens ////////
vm.warp(block.timestamp + 2 days);
​
vm.startPrank(account1);
TST.approve(pool, account1TstBalance);
EUROs.approve(pool, account1EurosBalance);
liquidationPool.increasePosition(account1TstBalance, account1EurosBalance);
vm.stopPrank();
​
vm.startPrank(account2);
TST.approve(pool, account2TstBalance);
EUROs.approve(pool, account2EurosBalance);
liquidationPool.increasePosition(account2TstBalance, account2EurosBalance);
vm.stopPrank();
​
vm.warp(block.timestamp + 2 days);
​
// Assert LiquidationPool received the deposits
assertEq(EUROs.balanceOf(pool), account1EurosBalance * 2, "TEST 5");
assertEq(TST.balanceOf(pool), account1TstBalance * 2, "TEST 6");
​
// starting prices: EUR/USD $11037; ETH/USD $2200; BTC/USD $42000; PAXGUSD $2000
setPriceAndTime(11037, 1100, 20000, 1000); // Drop collateral value
​
//////// Liquidate vault ////////
​
// struct Reward { bytes32 symbol; uint256 amount; uint8 dec; }
// Position { address holder; uint256 TST; uint256 EUROs; }
​
// Account1 pre-liquidation Position
ILiquidationPool.Position memory account1Position0;
ILiquidationPool.Reward[] memory account1Reward0 = new ILiquidationPool.Reward[](3);
(account1Position0, account1Reward0) = liquidationPool.position(account1);
​
uint256 EurosPosition = account1Position0.EUROs;
assertEq(account1EurosBalance, EurosPosition, "TEST 7");
​
// Bug fix: Without granting pool BURNER_ROLE, distributeAssets() reverts
vm.startPrank(SmartVaultManager);
IEUROs(euros_).grantRole(IEUROs(euros_).BURNER_ROLE(), pool);
vm.stopPrank();
​
vm.startPrank(liquidator);
liquidationPoolManagerContract.runLiquidation(1);
vm.stopPrank();
​
// account rewards
ILiquidationPool.Position memory account1Position1;
ILiquidationPool.Reward[] memory account1Reward1 = new ILiquidationPool.Reward[](3);
(account1Position1, account1Reward1) = liquidationPool.position(owner1);
​
// Assert account1 EUROs Position is wiped
uint256 EurosPosition1 = account1Position1.EUROs;
assertEq(EurosPosition1, 0, "TEST 8");
​
// Assert account1 receive no Reward
assertEq(account1Reward0[0].amount, account1Reward1[0].amount, "TEST 9");
assertEq(account1Reward0[1].amount, account1Reward1[1].amount, "TEST 10");
assertEq(account1Reward0[2].amount, account1Reward1[2].amount, "TEST 11");
}


Tools Used:

  • Manual review

  • Foundry


Recommended Mitigation Steps:
Change 10e10 to 1e10 in the costInEuros calculation.

if (asset.amount > 0) {
(,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;
+ uint256 costInEuros = _portion * 1 ** (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);
​
// Only comment the above line and uncomment this one when running testAssetsDistribution
IERC20(asset.token.addr).safeTransferFrom(msg.sender, address(this), _portion);
}
}
Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

informational/invalid

Support

FAQs

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