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

Participants will not get rewards/assets after liquidation process as being promised

bhilare71

Summary

Participants in staking will not get rewards/assets after liquidation process as being promised. Implementation of distribution logic seems to be flawed.

Vulnerability Details

Following lines are of Whitepaper of this protocol:

The distribution of assets during the liquidation process relies on the proportion of each participant's combined stake (sEURO and
TST) to the total combined stake in the liquidation pool.
​
2.8.2 Distribution Calculation
​
To calculate the distribution percentages for each participant, the following formula is used:
Distribution % = (Participant's sEURO + Participant's TST) / (Total sEURO + Total TST)
This formula ensures a fair distribution of assets based on each participant's contribution to the
liquidation pool in terms of sEURO and TST tokens.
​

As we can see here , its clearly stated that the participants will get the distribution of assets based on the combined stake i.e sEURO + TST , to the total combined stake.

Now for this part, relevant code in the codebase is :

Code
​
function stake(Position memory _position) private pure returns (uint256) {
return _position.TST > _position.EUROs ? _position.EUROs : _position.TST;
}
​
function getStakeTotal() private view returns (uint256 _stakes) {
for (uint256 i = 0; i < holders.length; i++) {
Position memory _position = positions[holders[i]];
_stakes += stake(_position);
}
}
​
​
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)
{
(,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;
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);
}

Now from the code , we can understand that while distributing assets , the portion of holder , presumably the proportion of distribution is being calculated by this line:

uint256 _portion = asset.amount * _positionStake / stakeTotal;

Now the _positionStake is the stake of individual participant which is obtained from calling stake function and stakeTotal is the total stake in the liquidity pool which is obtained from calling getStakeTotal() function.

uint256 _positionStake = stake(_position);
uint256 stakeTotal = getStakeTotal();

But the ambiguity lies in the stake & getStakeTotal function.

In stake function there's this statement using ternary operator

return _position.TST > _position.EUROs ? _position.EUROs : _position.TST;

Which means : if TST stake is greater than EUROs stake , then return EUROs else return TST.

And in getStakeTotal function uses this stake function itself for calculating the total stake in pool.

And before stating the ambiguity, according to the testcases done in the Test file of LiquidationPool contract. Participants are allowed to stake using either one of TST or EUROs OR both .

Relevant code is provided below from the test file, which does passes test while running the test file.

Test code
describe('increase position', async () => {
it('allows increasing position by one or both assets', async () => {
const balance = ethers.utils.parseEther('5000');
const tstVal = ethers.utils.parseEther('1000');
const eurosVal = ethers.utils.parseEther('500');
​
await TST.mint(user1.address, balance);
await EUROs.mint(user1.address, balance);
let increase = LiquidationPool.increasePosition(tstVal, eurosVal);
await expect(increase).to.be.revertedWith('ERC20: insufficient allowance')
​
let { _position} = await LiquidationPool.position(user1.address);
expect(_position.TST).to.equal('0');
expect(_position.EUROs).to.equal('0');
​
await TST.approve(LiquidationPool.address, tstVal);
await EUROs.approve(LiquidationPool.address, eurosVal);
​
increase = LiquidationPool.increasePosition(tstVal, eurosVal);
await expect(increase).not.to.be.reverted;
​
({_position} = await LiquidationPool.position(user1.address));
expect(_position.TST).to.equal(tstVal);
expect(_position.EUROs).to.equal(eurosVal);
​
await TST.approve(LiquidationPool.address, tstVal);
increase = LiquidationPool.increasePosition(tstVal, 0);
await expect(increase).not.to.be.reverted;
​
({_position} = await LiquidationPool.position(user1.address));
expect(_position.TST).to.equal(tstVal.mul(2));
expect(_position.EUROs).to.equal(eurosVal);
​
await EUROs.approve(LiquidationPool.address, eurosVal);
increase = LiquidationPool.increasePosition(0, eurosVal);
await expect(increase).not.to.be.reverted;
({_position} = await LiquidationPool.position(user1.address));
expect(_position.TST).to.equal(tstVal.mul(2));
expect(_position.EUROs).to.equal(eurosVal.mul(2));
});

Now the ambiguity part:

In stake function , only either value of TST or EUROs is being used . Which further is used in getStakeTotal function.

For example:

Now lets say A participant stakes 100 TST and 70 EUROS, the stake function, while calculating his portion , since TST> EURO, it will return EUROs value.

Now lets say B participant stakes 50 TST and 0 EUROS, the 'stake' function will return EUROs value , i.e 0.

Now lets say C participant stakes 100 TST and 120 EUROS, the stake function will return TST value, i.e 100.

Here the getStakeTotal() function will return (70 + 0 + 100) = 170.

Issues :

  1. B participant will get no portion of distribution of assets ,since stake function returned 0, even if he/she staked 50 TST in the pool as it was allowed as we saw in the test cases above. Hence getting NO rewards. Where he/she should have got (50+0)/(100+70+0+50+100+120) ~= 11.3637 % based on the combined stake.

  2. According to the protocol, A participant should get (100+70)/(100+70+0+50+100+120) = 38.6364 % of distribution, but according to the code it will get 70/170 ~= 41.1764 % of distribution.

  3. And for C, according to whitepaper => (100+120)/(100+70+0+50+100+120) = 50 %, but according to the code will get 100/170 ~= 58.82

This leads to uneven distribution of rewards where B is not getting any reward but A & C are getting more than they deserved according to their stake.

Also , participants are getting no benefit of staking more of TST or EUROs as the stake function is returning only the smaller one among them.

And they have also stated in whitepaper that liquidity staking should be done on 1:1 ratio of TST & EUROs or TST should be greater than EUROs, but in code base as we saw in the test cases, any one of both , staking is allowed. SO THIS WAS ALSO A POINT OF AMBUIGUITY .

Impact

Because of this issue, participants will not get fair amount of rewards as they are being promised. Also if deposited only one of the TST or EURO, they will not get any portion of the distribution asset.

This would be unfair for the participants because of which they may also lose trust, heavily impacting the protocol.

Tools Used

Thorough manual reviewing of code was being done.

Recommendations

The fix for this issue would be to update the stake function to correctly calculate the combined stake of a participant (both TST and EUROS stakes together) as being stated in the whitepaper, rather than just the smaller of the two stakes. This would ensure a fair distribution of rewards to all participants, regardless of how they choose to stake their tokens

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

informational/invalid

Support

FAQs

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

Give us feedback!