Incorrect calculation of fees in amount variable

Summary

The calculation of amount variable is incorrect, as it adds the fees for both the auction and the teller to the auction and liquidity amounts. This could result in the calculation of an incorrect amount, which could cause unexpected behavior in the auction or teller contracts.

Vulnerability Detail

https://github.com/sherlock-audit/2023-02-bond/blob/main/bonds/src/BondBatchAuctionV1.sol#L129

Assume, we have an auction contract and a teller contract, and we want to execute a trade where the buyer buys 100 tokens from the seller.

The auction contract charges a 1% fee on the trade, while the teller contract charges a 2% fee.

If the calculation of amount variable is incorrect and adds the fees for both contracts to auction and liquidity amounts, then the calculation will be as follows:

Auction amount: 100 tokens
Auction fee: 1 token (1% of 100 tokens)
Liquidity amount: 100 tokens
Liquidity fee: 2 tokens (2% of 100 tokens)
Total amount: 203 tokens (100 tokens + 1 token + 100 tokens + 2 tokens)

However, the correct calculation should only add the fees to amount that is paid by the buyer. Therefore, the correct calculation should be:

Auction amount: 100 tokens
Auction fee: 1 token (1% of 100 tokens)
Liquidity amount: 100 tokens
Liquidity fee: 2 tokens (2% of 100 tokens)
Total amount: 103 tokens (100 tokens + 1 token + 2 tokens)

Impact

If the incorrect calculation is used, it could result in unexpected behavior in the auction or teller contracts, such as incorrect token balances or failed trades. Therefore, it is important to ensure that the calculation of the amount variable is correct and takes into account only the fees that are paid by the buyer.

Code Snippet

Tool used

Manual Review

Recommendation

Separate the calculation of auction fees and teller fees from the calculation of amount. This will ensure that the fees are not mistakenly added to the amount, leading to incorrect calculations.

The fees are expected to be applied as follows:

auctionAmount needs both the teller and gnosis auction fees applied to it since the underlying gets sent to the teller to create payout tokens (teller fee taken) and then payout tokens are sent to the gnosis auction contract (where it takes its fee). This is calculated by amountWithFee (both fees are applied and in the order/manner which they are taken).

liquidityAmount only needs the teller fee applied since this underlying amount gets sent to the teller for create payout tokens (teller fee taken) and then sent back to market creator. This is calculated with amountWithTellerFee (only teller fee applied).

I disagree with this finding. I believe the implementation is correct.

sherlock-audit / 2023-02-bond-judging

OCC - Incorrect calculation of fees in amount variable #31