-
Notifications
You must be signed in to change notification settings - Fork 4
Open
Labels
🤖_primaryAI based primary recommendationAI based primary recommendationQA (Quality Assurance)Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntaxAssets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntaxbugSomething isn't workingSomething isn't workingdowngraded by judgeJudge downgraded the risk level of this issueJudge downgraded the risk level of this issuegrade-aprimary issueHighest quality submission among a set of duplicatesHighest quality submission among a set of duplicatessponsor confirmedSponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Description
Lines of code
https://github.com/code-423n4/2024-05-bakerfi/blob/main/contracts/core/Vault.sol#L260-L264
Vulnerability details
Description
The Vault::withdraw() function rounds-down the fee against the protocol's favour and hence a user can split their withdraw tx into multiple small ones such that fee evaluates to zero in each call. On less expensive chains like Arbitrum or Optimism, this strategy would be beneficial for them.
// Withdraw ETh to Receiver and pay withdrawal Fees
if (settings().getWithdrawalFee() != 0 && settings().getFeeReceiver() != address(0)) {
@----> fee = (amount * settings().getWithdrawalFee()) / PERCENTAGE_PRECISION;
payable(msg.sender).sendValue(amount - fee);
payable(settings().getFeeReceiver()).sendValue(fee);Proof of Concept
- Assume
settings().getWithdrawalFee()to be1e4. PERCENTAGE_PRECISIONis defined by the protocol as1e9.- Scenario1 (normal user):
amount= 1e5- Will have to pay a fee of
(1e5 * 1e4) / 1e9 = 1
- Scenario2 (malicious user):
- Using 2 txs of
0.5e5each - In each tx
amount= 0.5e5 - In each tx will have to pay a fee of
(0.5e5 * 1e4) / 1e9 = 0
Hence no fee paid by the malicious user.
- Using 2 txs of
Impact
Loss of fee for the protocol
Tools Used
Manual review
Recommended Mitigation Steps
Round up in favour of the protocol. A library like solmate can be used which has mulDivUp:
- fee = (amount * settings().getWithdrawalFee()) / PERCENTAGE_PRECISION;
+ fee = amount.mulDivUp(settings().getWithdrawalFee(), PERCENTAGE_PRECISION);Assessed type
Math
Metadata
Metadata
Assignees
Labels
🤖_primaryAI based primary recommendationAI based primary recommendationQA (Quality Assurance)Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntaxAssets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntaxbugSomething isn't workingSomething isn't workingdowngraded by judgeJudge downgraded the risk level of this issueJudge downgraded the risk level of this issuegrade-aprimary issueHighest quality submission among a set of duplicatesHighest quality submission among a set of duplicatessponsor confirmedSponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")