Basin - CRIMSON-RAT-REACH's results

Lines of code

https://github.com/code-423n4/2023-07-basin/blob/f15fe66d57c2f226c232685d16f297e54bcc0939/src/Well.sol#L562

Vulnerability details

Impact

The removeLiquidityImbalanced function in the Well.sol contract is vulnerable to a potential underflow. This could disrupt the contract's functionality and prevent users from removing liquidity in an imbalanced manner. Furthermore, the function does not appropriately handle cases where reserves can be greater than minted liquidity due to external factors like donations. This could lead to unnecessary reverts of legitimate removeLiquidity transactions.

Proof of Concept

The issue is located in the removeLiquidityImbalanced function of the Well.sol contract, specifically at this line: Well.sol#L562.

The potential underflow occurs in the following line of code:

lpAmountIn = totalSupply() - _calcLpTokenSupply(wellFunction(), reserves);

In addition, the function doesn't handle the scenario where reserves can be greater than minted liquidity due to external factors like donations. This could lead to unnecessary reverts of legitimate removeLiquidity transactions.

Tools Used

The code review and analysis were performed manually, without the use of any specific security tools.

Recommended Mitigation Steps

To mitigate this issue, it is recommended to add a check to ensure totalSupply() is not less than the return value of _calcLpTokenSupply. If it is, lpTokenSupply should be set to totalSupply(). This change would make the function call complete successfully even when reserves can be greater than minted liquidity due to donations. Here is an example of how this could be implemented:

uint256 lpTokenSupply = _calcLpTokenSupply(wellFunction(), reserves);
if (totalSupply() < lpTokenSupply) {
    lpTokenSupply = totalSupply();
}
lpAmountIn = totalSupply() - lpTokenSupply;

This change will prevent the underflow from happening and also account for the scenario where reserves are greater than the total supply of liquidity tokens. This allows the function to handle donations and similar scenarios without reverting legitimate removeLiquidity transactions.

Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-07-basin/blob/f15fe66d57c2f226c232685d16f297e54bcc0939/src/Well.sol#L238

Vulnerability details

Description

The contract has several events that are not marked as indexed. This can result in inefficient event filtering and querying, as well as potential limitations in functionality for applications built on top of this contract.

Impact

Without indexing, event filtering becomes resource-intensive, hindering scalability and impeding the contract's ability to handle a high volume of events. Additionally, the absence of indexing limits the functionality and flexibility of applications built on top of the contract, making advanced event-based queries and operations challenging. This inefficiency increases computational resources, transaction costs, and raises compliance and audit concerns, potentially resulting in legal and reputational risks for the contract deployer. Promptly addressing this critical issue is crucial to ensure optimal performance, usability, and regulatory compliance of the contract.

Proof of Concept

N/A

Tools Used

Manual Review

Recommended Mitigation Steps

Add the indexed keyword in each event.

Assessed type

Other