Basin - Kaysoft's results

[L-01] SafeCast library is unused in Aquifer.sol. Remove unused Library.

The SafeCast library in the Aquifer.sol contract is not used.

File: https://github.com/code-423n4/2023-07-basin/blob/c1b72d4e372a6246e0efbd57b47fb4cbb5d77062/src/Aquifer.sol#L20

contract Aquifer is IAquifer, ReentrancyGuard {
    using SafeCast for uint256;//@audit remove unused library.
    using LibClone for address;
...

Recommended Mitigation Steps

Consider removing the SafeCast library since it is not used.

[L-02] Avoid using the draft draft-ERC20PermitUpgradeable.sol openzeppelin

The Well.sol contract imports the draft draft-ERC20PermitUpgradeable.sol file of openzeppelin contracts which is also marked as deprecated in the file when checked. This Openzepelin's warning about the draft directory: https://docs.openzeppelin.com/contracts/3.x/api/drafts

"This directory contains implementations of EIPs that are still in Draft status.

Due to their nature as drafts, the details of these contracts may change and we cannot guarantee their stability. Minor releases of OpenZeppelin Contracts may contain breaking changes for the contracts in this directory, which will be duly announced in the changelog. The EIPs included here are used by projects in production and this may make them less likely to change significantly."

File: https://github.com/code-423n4/2023-07-basin/blob/c1b72d4e372a6246e0efbd57b47fb4cbb5d77062/src/Well.sol#L6

import {ERC20Upgradeable, ERC20PermitUpgradeable} from "ozu/token/ERC20/extensions/draft-ERC20PermitUpgradeable.sol";

Recommended Mitigation Steps

Consider using the ERC20PermitUpgradeable.sol file since it is available instead of importing the draft.

[L-03] Avoid variable shadowing

The name and symbol parameters of the init() function of the Well.sol contract shadows the ERC20Upgradable.name and ERC20Upgradable.symbol. The ERC20Upgradable is a parent contract inherited by ERC20PermitUpgradeable.

File: https://github.com/code-423n4/2023-07-basin/blob/c1b72d4e372a6246e0efbd57b47fb4cbb5d77062/src/Well.sol#L32

function init(string memory name, string memory symbol) public initializer {
        __ERC20Permit_init(name);
        //@audit shadowing name and symbol of ERC20
        __ERC20_init(name, symbol);

        IERC20[] memory _tokens = tokens();
        for (uint256 i; i < _tokens.length - 1; ++i) {
            for (uint256 j = i + 1; j < _tokens.length; ++j) {
                //@audit double for loop
                if (_tokens[i] == _tokens[j]) {
                    revert DuplicateTokens(_tokens[i]);
                }
            }
        }
    }

Recommended Mitigation Steps

Consider renaming the name and symbol parameters of the init functions and avoid shadowing another variable.

[L-04] The Clone.sol and ClonePlus.sol contracts have the same state variable names ONE_WORD.

uint256 private constant ONE_WORD = 0x20; The above state variable is defined in both the Clone.sol and ClonePlus.sol and ClonePlus.sol inherits the Clone.sol contract. ClonePlus.sol contract is in turn a parent contract for Well.sol contract.

Files:

• https://github.com/code-423n4/2023-07-basin/blob/c1b72d4e372a6246e0efbd57b47fb4cbb5d77062/src/utils/Clone.sol#L9
• https://github.com/code-423n4/2023-07-basin/blob/c1b72d4e372a6246e0efbd57b47fb4cbb5d77062/src/utils/ClonePlus.sol#L10

uint256 private constant ONE_WORD = 0x20; //@audit same variable name in Clone and ClonePlus.

Recommended Mitigation Steps

Consider removing the ONE_WORD variable from one of the Clone.sol ClonePlus.sol