search

code-423n4 / 2022-01-xdefi-findings

0 stars 0 forks source link

Possible profitability manipulations #193

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

Czar102

Vulnerability details

Impact

An owner of the contract may, by ordering their or others' locking transactions, significantly increase or decrease bonusMultiplier for some set of transactions.

So, by mining a single block, an owner can lower other's bonus multipliers, execute locking transactions and then restore bonus multipliers.

A user might send a locking transaction in a similar time as an owner lowers the multipliers, resulting in lowering the revenue against data presented to the user.

An owner can also pass ownership to a contract that will change bonus multipliers and lock funds with a very high bonus multiplier, then restore previous multipliers' state not to let others do the same. This way, the owner can gain an unfair advantage over others.

Recommended Mitigation Steps

Use a timelock for setLockPeriods(...) function and require passing bonusMultiplier in locking functions, revert if they are different from the state variables.

deluca-mike commented 2 years ago

It is true that the owner of the contract can set, reset, and delete lock periods and bonuses, but keep in mind that the owner is the XDEFI organization, who is also airdropping the rewards, so there is some trust needed, which is why I categorize this as low risk, since it makes the assumption that the XDEFI organization that is giving out rewards wants to act poorly and has the ability to order transactions in a block.

I agree with the need for the bonusMultiplier as a argument for users, so that it can be confirmed at the moment their transaction gets mined.

I do not agree with a timelock for changing bonus multipliers, since that requires a lot more state and calls, and just makes things more complicated when the attack assumptions are already unlikely.

deluca-mike commented 2 years ago

In the release candidate contract, all locking functions take the bonusMultiplier as a argument, and check that applied bonusMultiplier is greater or equal to what the user expected.

Ivshti commented 2 years ago

agreed with the mitigation and lowering of severity