Frax Ether Liquid Staking contest - wagmi's results

Lines of code

https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/sfrxETH.sol#L84

Vulnerability details

Impact

Function sfrxETH.mintWithSignature() allows users to approve and mint in one transaction. Users will provide input param shares and function will calculate what assets amount needed to be approved (in case approveMax = false).

uint256 amount = approveMax ? type(uint256).max : previewMint(shares);
asset.permit(msg.sender, address(this), amount, deadline, v, r, s);
return (mint(shares, receiver));

It used previewMint() of ERC4626 to calculate amount from shares input param. Users need to sign a message with this amount value off-chain.

But since in design, the exchange rate of sfrxETH will increase linearly over time (every second), there is no way users can calculate the exact value of amount from share off-chain. For example, users calculate amount = X given shares = Y off-chain and sign amount = X. But when transaction is executed, the exchange rate is changed, amount > X given shares = Y.

The result is user’s signature invalid and cannot be executed.

Proof of Concept

Consider the scenario

1. Alice wants to approve and mint 100 sfrxETH in 1 transaction. She got the exchange rate from the contract 1 frxETH = 1 sfrxETH. So she signs amount = 100 to approve 100 frxETH. Note that Alice doesn’t want to approve max since it’s more risky and not recommended from her expert crypto friends.

2. She sends the transaction and waits for it to be executed. But by the time it’s executed, the exchange rate is updated 1.1 frxETH = 1 sfrxETH, which means if she wants to mint 100 sfrxETH she needs to approve 110 frxETH. So her transaction is reverted.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider add 1 more input param amount in mintWithSignature() function. Or only allow users to approve max.

Summary

1. Redundant zero initialization

Solidity does not recognize null as a value, so uint variables are initialized to zero. Setting a uint variable to zero is redundant and can waste gas.

There are several places where an int is initialized to zero, which looks like:

withholdRatio = 0;

Affected Codes

• https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/frxETHMinter.sol#L63
• https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/frxETHMinter.sol#L64
• https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/frxETHMinter.sol#L94
• https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/frxETHMinter.sol#L129
• https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/OperatorRegistry.sol#L63
• https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/OperatorRegistry.sol#L84
• https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/OperatorRegistry.sol#L114
• https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/ERC20/ERC20PermitPermissionedMint.sol#L84

2. Short require strings save gas

Strings in solidity are handled in 32 byte chunks. A require string longer than 32 bytes uses more gas. Shortening these strings will save gas.

One cases of this gas optimization was found 35 chars https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/frxETHMinter.sol#L167

3. Use != 0 instead of > 0

Using > 0 uses slightly more gas than using != 0. Use != 0 when comparing uint variables to zero, which cannot hold values below zero

Affected Codes

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