Summary:
safeApprove is deprecated by Openzeppelin
Vulnerability Details
In Ethereum and ERC-20 tokens, the safeApprove function was introduced as a way to address potential security risks associated with the re-use of approval tokens in decentralized finance (DeFi) applications. However, it has been deprecated due to concerns over its effectiveness and the complexity it introduces into smart contract interactions.
The safeApprove function was intended to prevent issues where changing an approval amount could be exploited if the previous amount was not first set to zero. However, the function was found to introduce potential race conditions and added complexity, which could itself lead to security vulnerabilities.
Multiple instances have been identified in the codebase where the safeApprove function from OpenZeppelin's contracts, which is deprecated, is being utilized.
Instance 1:
https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyArb.sol#L27
constructor(
address _asset,
address _transmuter,
string memory _name
) BaseStrategy(_asset, _name) {
transmuter = ITransmuter(_transmuter);
require(transmuter.syntheticToken() == _asset, "Asset does not match transmuter synthetic token");
@> underlying = ERC20(transmuter.underlyingToken());
asset.safeApprove(address(transmuter), type(uint256).max);
_initStrategy();
}
Instance 2:
https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyArb.sol#L37
function _initStrategy() internal {
router = 0xAAA87963EFeB6f7E0a2711F397663105Acb1805e;
@> underlying.safeApprove(address(router), type(uint256).max);
}
Instance 3:
https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyArb.sol#L44
function setRouter(address _router) external onlyManagement {
router = _router;
@> underlying.safeApprove(router, type(uint256).max);
}
Instance 4:
https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyMainnet.sol#L35
constructor(
address _asset,
address _transmuter,
string memory _name
) BaseStrategy(_asset, _name) {
transmuter = ITransmuter(_transmuter);
require(transmuter.syntheticToken() == _asset, "Asset does not match transmuter synthetic token");
underlying = ERC20(transmuter.underlyingToken());
@> asset.safeApprove(address(transmuter), type(uint256).max);
_initStrategy();
}
Instance 5:
https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyMainnet.sol#L45
function _initStrategy() internal {
router = ICurveRouterNG(0xF0d4c12A5768D806021F80a262B4d39d26C58b8D);
@> underlying.safeApprove(address(router), type(uint256).max);
}
Instance 6:
https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyOp.sol#L27
constructor(
address _asset,
address _transmuter,
string memory _name
) BaseStrategy(_asset, _name) {
transmuter = ITransmuter(_transmuter);
require(transmuter.syntheticToken() == _asset, "Asset does not match transmuter synthetic token");
underlying = ERC20(transmuter.underlyingToken());
@> asset.safeApprove(address(transmuter), type(uint256).max);
_initStrategy();
}
Instance 7:
function _initStrategy() internal {
router = 0xa062aE8A9c5e11aaA026fc2670B0D65cCc8B2858;
@> underlying.safeApprove(address(router), type(uint256).max);
}
Instance 8:
https://github.com/Cyfrin/2024-12-alchemix/blob/82798f4891e41959eef866bd1d4cb44fc1e26439/src/StrategyOp.sol#L50
function setRouter(address _router) external onlyManagement {
router = _router;
@> underlying.safeApprove(router, type(uint256).max);
}
Recommendations
Its recommended to use safeIncreaseAllowance and safeDecreaseAllowance instead whenever possible. For more information about the recommendations check https://docs.openzeppelin.com/
