Aquifer.sol-Well.sol - potential well with duplicate tokens

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-basin/blob/c1b72d4e372a6246e0efbd57b47fb4cbb5d77062/src/Aquifer.sol#L54-L64 https://github.com/code-423n4/2023-07-basin/blob/c1b72d4e372a6246e0efbd57b47fb4cbb5d77062/src/Well.sol#L31-L43

Vulnerability details

Impact

In the current Basin implementation, deployed Well contracts operate as 'guilty until proven innocent', leaving users at their own risk about which Wells to use, but providing them with a list of verified wells. In a past audit report of the protocol an issue was reported regarding the possible creation of a Well with duplicate tokens, which could be exploited by skimming the excess after a deposit. Even though a Well of this type should be avoided by the users, it is still an unwanted behavior, so the issue was aknowledged, but the provided fix doesn't apply in some scenarios. I provide a link to the original finding: https://basin.exchange/cyfrin-basin-audit.pdf Issue: 6.2.1 The problem arises, because of the possibility of not invoking the init function of a Well if no initFunctionCall data is provided to the Aquifer, leading to the check for duplicate tokens not occuring

Proof of Concept

A user can just provided no initFunctioncCall data, which would lead to the POC and impact of the previous audit, the one provided in https://basin.exchange/cyfrin-basin-audit.pdf 6.2.1 is well explained

Tools Used

Manual Review

Recommended Mitigation Steps

Add a constructor to the well to do the needed check, so that it cannot be skipped over.

I understand that users are incentivized to do their research, but this is unintended behavior by the protocol, that was already aknowledged as an issue in the past. I will understand if this gets downgraded.

Assessed type

ERC20

[c4-pre-sort]

141345 marked the issue as low quality report

[c4-pre-sort]

141345 marked the issue as duplicate of #182

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)