Submission Details
Severity:
wrong `msg.sender` passed as router
Summary
when a user wants to remove liquidity if adminFeePercent != 1e18 then a portion of the fee paid goes back to the router address but the wrong msg.sender is passed adding liquidity to the vault instead lof ocking the fees
Vulnerability Details
when a user removes liquidity a onAfterRemoveLiquidity that adds fees as liquidity back to the pool
File: UpliftOnlyExample.sol
555: if (localData.adminFeePercent != 1e18) {
556: // Donates accrued fees back to LPs.
557: _vault.addLiquidity(
558: AddLiquidityParams({
559: pool: localData.pool,
560:@> to: msg.sender, // It would mint BPTs to router, but it's a donation so no BPT is minted
561: maxAmountsIn: localData.accruedFees, // Donate all accrued fees back to the pool (i.e. to the LPs)
562: minBptAmountOut: 0, // Donation does not return BPTs, any number above 0 will revert
563: kind: AddLiquidityKind.DONATION,
564: userData: bytes("") // User data is not used by donation, so we can set it to an empty string
565: })
566: );
POC
add this in UpliftExample.t.sol
function testRemoveLiquidityWrongMsg_Sender() public {
vm.prank(address(vaultAdmin));
updateWeightRunner.setQuantAMMUpliftFeeTake(0.5e18);
vm.stopPrank();
// Add liquidity so bob has BPT to remove liquidity.
uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
vm.prank(bob);
upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
vm.stopPrank();
uint256 nftTokenId = 0;
uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
vm.startPrank(bob);
upliftOnlyRouter.removeLiquidityProportional(bptAmount, minAmountsOut, false, pool);
vm.stopPrank();
BaseVaultTest.Balances memory balancesAfter = getBalances(updateWeightRunner.getQuantAMMAdmin());
}
address of the router is passed when adding liquidity appears as follows
├─ [436383] upliftOnlyRouter::addLiquidityProportional(pool: [0xDB25A7b768311dE128BBDa7B8426c3f9C74f3240], [1000000000000000000000000000 [1e27], 1000000000000000000000000000 [1e27]], 2000000000000000000000 [2e21], false, 0x)
│ ├─ [180528] vault::unlock(0xe41535c200000000000000000000000000000000000000000000000000000000000000200000000000000000000000001d96f2f6bef1202e4ce1ff6dad0c2cb002861d3e000000000000000000000000f2e246bb76df876cef8b38ae84130f4f55de395b000000000000000000000000db25a7b768311de128bbda7b8426c3f9c74f3240000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000006c6b935b8bbd40000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000033b2e3c9fd0803ce80000000000000000000000000000000000000000000000033b2e3c9fd0803ce80000000000000000000000000000000000000000000000000000000000000000000000)
│ │ ├─ [177822] upliftOnlyRouter::addLiquidityHook(ExtendedAddLiquidityHookParams({ sender: 0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e, receiver: 0xF2E246BB76DF876Cef8b38ae84130F4F55De395b,
As the receiver address
while removing liquidity msg.sender is as follow
// first add liquidity for quantammadmin
├─ [281] updateWeightRunner::getQuantAMMAdmin() [staticcall]
│ │ │ │ │ │ └─ ← [Return] vaultAdmin: [0x212224D2F2d262cd093eE13240ca4873fcCBbA3C]
│ │ │ │ │ ├─ [85100] vault::addLiquidity(AddLiquidityParams({ pool: 0xDB25A7b768311dE128BBDa7B8426c3f9C74f3240, to: 0x212224D2F2d262cd093eE13240ca4873fcCBbA3C, maxAmountsIn: [250000000000000000 [2.5e17], 250000000000000000 [2.5e17]], minBptAmountOut: 500000000000000000 [5e17], kind: 0, userData: 0x }))
// 2nd add liquidity with wrong msg.sender
│ │ │ │ │ ├─ [56324] vault::addLiquidity(AddLiquidityParams({ pool: 0xDB25A7b768311dE128BBDa7B8426c3f9C74f3240, to: 0xB67aBC332D1f48fB59f599d315B6c621e234A4c9, maxAmountsIn: [250000000000000000 [2.5e17], 250000000000000000 [2.5e17]], minBptAmountOut: 0, kind: 3, userData: 0x }))
Impact
wrong (msg.sender) used
wrong minted values
broken function and not working as intended
Tools Used
manual review
Recommendations
adjust the address to be address(this) instead of msg.sender
Prize pool breakdown
Total prize
49,600 OP
nSLOC
3,00017
OP / LOC
44,640
4,960
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.