QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: medium
Invalid

onlySelfRouter modifier is not guarding against anything

octeezy

Summary

The onlySelfRouter modifier in the UpliftOnlyExample.sol contract is not guarding against anything when applied to the onAfterRemoveLiquidity function as it's being passed the router address as an argument. This means that anyone can execute this function as long as they pass the UpliftOnlyExample address as the first argument.

Vulnerability Details

Basic PoC, paste this at the end of the UpliftExample.t.sol file.

function testPoCMinLiquidityAdded() public {
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
vm.prank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
vm.stopPrank();
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
​
// Execute as Real depositor
vm.startPrank(bob);
upliftOnlyRouter.onAfterRemoveLiquidity(
address(upliftOnlyRouter),
pool,
RemoveLiquidityKind.PROPORTIONAL,
bptAmount,
minAmountsOut,
minAmountsOut,
minAmountsOut,
bytes("")
);
vm.stopPrank();
}

This test is supposed to fail with the CannotUseExternalRouter selector as an external account is executing the function. It doesn't due to the correct uplift router being passed as an argument.

NOTE: the router variable is not used inside the onAfterRemoveLiquidity function besides being passed to the onlySelfRouter modifier.

Impact

Lack of Authorization guard

Tools Used

Manual review + foundry tests

Recommendations

If this is supposed to guard against external use consider changing onlySelfRouter(router) to onlySelfRouter(msg.sender) in UpliftOnlyExample.sol#L440

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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

Give us feedback!