Tadle

Summary

The core protocol functions, including closeOffer, relistOffer, abortBidTaker, and abortAskOffer, are vulnerable to reentrancy attacks. This vulnerability arises from not following the check-effects-interaction pattern, which could lead to significant financial losses.

Vulnerability Details

The closeOffer, relistOffer, abortBidTaker, and abortAskOffer functions do not adhere to the check-effects-interaction pattern, as illustrated below:

• The relistOffer function is susceptible to reentrancy attacks due to the order in which state changes and external calls are made. Here’s a breakdown of how reentrancy can be exploited in this function:

1. ** External Call Before State Update**: The function calls tokenManager.tillIn to transfer a refund token from the user to the capital pool before updating the offer's status to Virgin. This is a critical point because an external call (to the tillIn function) is made before changing the state of offerInfo.offerStatus.

2. Potential Attack Vector: If an attacker can control the msg.sender and has a malicious fallback function in the contract being called, they could re-enter the relistOffer function before the offerStatus is updated. This means they can potentially call relistOffer again while the offer status is still in a state that allows for further execution.

3. State Manipulation: Since the function allows for the offerStatus to be manipulated before the status is actually set to Virgin, an attacker could exploit this by calling the function multiple times, leading to unintended behavior such as multiple refunds or unauthorized state changes.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L470-L526

Exploit Scenario

User A calls relistOffer, and the function checks the conditions, then calculates the depositAmount.

• When tokenManager.tillIn is called, control is passed to an external contract (the token manager).

• If this external contract has a malicious fallback function that calls relistOffer again, the attacker can effectively call the function recursively.

• The function's state hasn't been updated yet, so it can still pass the checks, potentially leading to unauthorized access or exploitation of the capital pool.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L406-L460

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L536-L635

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L645-L697

• In the closeOffer function, refundAmount is added to the token balance of the user before updating its offerStatus. An attacker can exploit this by using a malicious contract to re-enter this function, leading to a loss of funds.

• In the abortAskOffer function, the refund amount for the offer authority is updated and added to the offer authority before the offer status is changed. The same attack vector can be utilized to drain protocol funds due to reentrancy.

• In the abortBidTaker function, transferAmount is added to the token balance of the authority before the stock status is changed. A malicious authority can re-enter this function to steal protocol funds.

Impact

Protocol funds will be stolen.

Recommendation

To prevent such reentrancy vulnerabilities, it is essential to follow the check-effects-interactions pattern. This means:

• Perform all checks first.

• Update all necessary state variables.

• Finally, make external calls.