codegpt - Deprecated safeApprove method

[sherlock-admin2]

codegpt

medium

Deprecated safeApprove method

Summary

safeApprove() function is deprecated and still being used in the MultiAccount contract.

Vulnerability Detail

safeApprove() function has been deprecated by OpenZeppelin team since v3.1.0:

• Deprecate SafeERC20.safeApprove OpenZeppelin/openzeppelin-contracts#2268
• https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/f4b309ae37dc650efa7523dd1230d230e3f452d7/CHANGELOG.md?plain=1#L584C51-L584C115

However, safeApprove() is still used in the following functions in MultiAccount contract:

• depositForAccount
• depositAndAllocateForAccount

Impact

Code Snippet

    function depositForAccount(
        address account,
        uint256 amount
    ) external onlyOwner(account, msg.sender) whenNotPaused {
        address collateral = ISymmio(symmioAddress).getCollateral();
        IERC20Upgradeable(collateral).safeTransferFrom(
            msg.sender,
            address(this),
            amount
        );
        IERC20Upgradeable(collateral).safeApprove(symmioAddress, amount);
        ISymmio(symmioAddress).depositFor(account, amount);
        emit DepositForAccount(msg.sender, account, amount);
    }

    function depositAndAllocateForAccount(
        address account,
        uint256 amount
    ) external onlyOwner(account, msg.sender) whenNotPaused {
        address collateral = ISymmio(symmioAddress).getCollateral();
        IERC20Upgradeable(collateral).safeTransferFrom(
            msg.sender,
            address(this),
            amount
        );
        IERC20Upgradeable(collateral).safeApprove(symmioAddress, amount);
        ISymmio(symmioAddress).depositFor(account, amount);
        bytes memory _callData = abi.encodeWithSignature(
            "allocate(uint256)",
            amount
        );
        innerCall(account, _callData);
        emit AllocateForAccount(msg.sender, account, amount);
        emit DepositForAccount(msg.sender, account, amount);
    }

https://github.com/sherlock-audit/2023-08-symmetrical/blob/main/symmio-core/contracts/multiAccount/MultiAccount.sol#L142C1-L176C6

Tool used

Manual Review

Recommendation

Recommend using safeIncreaseAllowance() and safeDecreaseAllowance() function instead.

[sherlock-admin]

2 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, because no funds loss and is just best practice advice which is invalid by sherlock rules

0xyPhilic commented:

invalid because it can be considered informational