code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/frxETHMinter.sol#L74-L75

Vulnerability details

Impact

When changing the allowance value from an existing non-zero value, certain tokens must first be approved by zero (before approving the actual allowance). Otherwise the token will not work. Since frxETH and sfrxETH are ERC20 compliant, it is possible that this problem will be triggered.

Proof of Concept

There is one instance of missing zero approval:

frxETHMinter.sol: L74-75

        // Approve frxETH to sfrxETH for staking
        frxETHToken.approve(address(sfrxETHToken), msg.value);

Tools Used

Manual analysis

Recommended Mitigation Steps

Set the allowance to zero before the approve() call, as follows:

        frxETHToken.approve(_to,0);
        // Approve frxETH to sfrxETH for staking
        frxETHToken.approve(address(sfrxETHToken), msg.value);

FortisFortuna commented 2 years ago

frxETH and sfrxETH are the only ERC20s used here and do not require approve(0) first.

0xean commented 2 years ago

closing as invalid.

code-423n4 / 2022-09-frax-findings