TempleGold

Summary

The audit reveals an issue in the removeAuctionConfig and recoverToken functions of the smart contract SpiceAuction, where the removal of an auction configuration or recovery of tokens may be improperly executed based on the current state of the auction. The conditions to execute these functions conflict with the intended operational logic, leading to potential unexpected behaviors and security risks.

Vulnerability Details

https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L107

https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/SpiceAuction.sol#L234

1. removeAuctionConfig Function:

   • The function cannot be executed after startAuction has been called, as the startTime is set by startAuction.

   • The current logic checks startTime == 0 to prevent execution if the auction has started, but this is inconsistent with the comment suggesting it should handle cases where auctionStart is triggered and cooldown is pending.

   • Key logic issues:

     • Checks if info.startTime == 0 to revert if the auction has started.

     • Reverts if the auction is active with info.isActive().

     • Confusing handling of configurations set but not started (auctionConfigs[id+1].duration > 0).

2. recoverToken Function:

   • This function has a comment suggesting the use of removeAuctionConfig when the auction is called but cooldown is pending, creating an inconsistency in the execution logic.

   • Key logic issues:

     • Reverts if info.startTime == 0, contradicting the comment.

     • Checks if the auction has not ended and the next epoch's duration is zero to revert with RemoveAuctionConfig.

Impact

The identified issues can lead to the following impacts:

• Improper Function Execution: Both removeAuctionConfig and recoverToken functions may be inappropriately restricted or allowed, leading to unexpected behaviors.

• Security Risks: Incorrect handling of auction states could allow unauthorized removal of auction configurations or unauthorized recovery of tokens, resulting in potential financial loss and breach of contract integrity.

Tools Used

Manual Review

Recommendations

1. Consistent Logic and Comments:

   • Ensure that the logic in removeAuctionConfig and recoverToken functions is consistent with the comments and intended functionality.

   • Update the comments to accurately reflect the operational checks and states being handled.

2. Enhanced State Checks:

   • Refactor the condition checks to clearly differentiate between states where an auction has started, is pending cooldown, or is not started yet.

   • Implement additional safeguards to handle edge cases where auction configurations might be inconsistently set or removed.

3. Comprehensive Testing:

   • Perform extensive testing to validate that the functions operate correctly under various auction states, including started, pending cooldown, and not started scenarios.

   • Ensure that all possible edge cases are covered and documented.

By addressing these recommendations, the smart contract can achieve the intended behavior for managing auction configurations and recovering tokens, while minimizing potential risks and vulnerabilities.