sherlock-audit / 2023-02-bond-judging

2 stars 0 forks source link

Bauer - The initiateBatchAuction transaction could fail #36

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Bauer

high

The initiateBatchAuction transaction could fail

Summary

The protocol misses check that if the minted bond tokens is less than the batchAuctionParams_.liquidityAmount , it could lead the transaction to fail.

Vulnerability Detail

The BondBatchAuctionV1.initiateBatchAuction() function is used to initiate auction. The protocol first validates bond token params. Then, deploy Bond Token (checks exist on the teller to see if it already exists). Next, transfer underlying token from owner to address(this) and call the teller().create() to mint bond tokens. After that,send the bond tokens reserved to provide liquidity to the sender if batchAuctionParams_.liquidityAmount > 0. The batchAuctionParams_.liquidityAmount comes from the parameters passed by the user. Here is the probolem, if the minted bond tokens is less than the batchAuctionParams_.liquidityAmount, the transaction will fail.

 if (batchAuctionParams_.liquidityAmount > 0)
                payoutToken.safeTransfer(owner(), uint256(batchAuctionParams_.liquidityAmount));
        }

Impact

If the minted bond tokens is less than the batchAuctionParams_.liquidityAmount, the transaction will fail.

Code Snippet

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

Tool used

Manual Review

Recommendation

Check the ```batchAuctionParams_.liquidityAmount````

Oighty commented 1 year ago

The amount of underlying that is sent to the teller to mint bond tokens from includes auctionAmount, liquidityAmount, and fees charged by the both the gnosis auction and teller contracts. The amount returned from the teller will be that total amount minus the teller fees (which were added on top of the inputs to ensure the correct amounts are received). Therefore, the contract should always have enough bond tokens so that this transfer fails, unless it failed earlier in the function.

UsmannK commented 1 year ago

Escalate for 10 USDC.

This issue is falsely marked as a dup of #39. However this issue and #39 are separate false issues.

For this issue, the sponsor explains how the watson report is incorrect.

Watson claims that

if the minted bond tokens is less than the batchAuctionParams_.liquidityAmount, the transaction will fail.

Code and developer show that this cannot be the case, as the minted tokens is calculated as such:

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

            // Calculate fee for minting bond tokens
            uint256 amount = amountWithFee(uint256(batchAuctionParams_.auctionAmount)) +
                amountWithTellerFee(uint256(batchAuctionParams_.liquidityAmount));

And so, the amount variable includes all relevant sub amounts (including batchAuctionParams_.liquidityAmount) and the proposed underflow will never occur.

sherlock-admin commented 1 year ago

Escalate for 10 USDC.

This issue is falsely marked as a dup of #39. However this issue and #39 are separate false issues.

For this issue, the sponsor explains how the watson report is incorrect.

Watson claims that

if the minted bond tokens is less than the batchAuctionParams_.liquidityAmount, the transaction will fail.

Code and developer show that this cannot be the case, as the minted tokens is calculated as such:

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

            // Calculate fee for minting bond tokens
            uint256 amount = amountWithFee(uint256(batchAuctionParams_.auctionAmount)) +
                amountWithTellerFee(uint256(batchAuctionParams_.liquidityAmount));

And so, the amount variable includes all relevant sub amounts (including batchAuctionParams_.liquidityAmount) and the proposed underflow will never occur.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

hrishibhat commented 1 year ago

Escalation accepted

Not a duplicate of #39

sherlock-admin commented 1 year ago

Escalation accepted

Not a duplicate of #39

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.