User can withdraw in multiple calls with small amount to escape fee

[c4-bot-2]

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 be 1e4.
• PERCENTAGE_PRECISION is defined by the protocol as 1e9.
• Scenario1 (normal user):
  • amount = 1e5
  • Will have to pay a fee of (1e5 * 1e4) / 1e9 = 1
• Scenario2 (malicious user):
  • Using 2 txs of 0.5e5 each
  • 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.

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

[0xleastwood]

Not reasonable to expect this to be profitable even with low gas costs.

[c4-judge]

0xleastwood marked the issue as primary issue

[0xleastwood]

Downgrading to QA for above reasons.

[c4-judge]

0xleastwood changed the severity to QA (Quality Assurance)

[t0x1cC0de]

Hi @0xleastwood, I had taken my inspiration from the live bug bounty payout of Graph protocol which paid out a considerable reward a few months back for a similar bug which allowed the user to save their curationTax and hence was considered a High. Here's the article outlining it and an excerpt from the article talking about the first of the two bugs:

Normally, high gas costs on mainnet would prevent this from becoming a lucrative attack. But because this target contract is also deployed on Arbitrum, an L2 with much cheaper gas, the attacker could successfully avoid paying the curationTax completely by minting needed amount of tokens in batches of 99 tokens per call as presented in the PoC created by @GregadETH.

This could essentially rob the protocol of owed revenue and cause loss of user funds or yield owed to protocol participants.

It's not only about the amount saved by the attacker, but the aspect that knowingly or unknowingly when mutliple users do so over a long period of time, there is value leakage for the protocol which accumulates over time.

Here's an example from a similar attack vector from the Code4rena Inverse Finance audit which talks about the economic feasibility/profitability of such attacks. The severity is still kept at Medium and not a QA.

Hence I personally believe it to be suitable for a Medium, but leave the final decision to you.

Thanks

[ickas]

Fixed → https://github.com/baker-fi/bakerfi-contracts/pull/51

[0xleastwood]

Regardless, i don't really see the feasibility of this even in a low gas cost environment. Not inclined to upgrade this to medium severity because there is negligible rounding.