The setApprovals() function of ConvexYieldWrapper.sol calls approve(spender, 0) for the curveToken but fails to do so for the convexToken. Some tokens, like USDT, require first reducing the address' allowance to zero by calling approve(spender, 0) before setting a new allowance. Since this contract is based off of a Convex project contract and the Convex contract calls approve(spender, 0) for the Convex token, it is recommended to follow the coding conventions used by a project managing its own token.
Proof of Concept
The setApprovals() function code:
function setApprovals() public {
IERC20(curveToken).approve(convexBooster, 0);
IERC20(curveToken).approve(convexBooster, type(uint256).max);
IERC20(convexToken).approve(convexPool, type(uint256).max);
}
Use the same approve(0) call for convexToken that is used for curveToken before using approve(type(uint256).max) to avoid any edge cases such as the ERC20 approve race condition.
alcueca
commented
2 years ago
Handle
sirhashalot
Vulnerability details
Impact
The
setApprovals()function of ConvexYieldWrapper.sol callsapprove(spender, 0)for the curveToken but fails to do so for the convexToken. Some tokens, like USDT, require first reducing the address' allowance to zero by callingapprove(spender, 0)before setting a new allowance. Since this contract is based off of a Convex project contract and the Convex contract callsapprove(spender, 0)for the Convex token, it is recommended to follow the coding conventions used by a project managing its own token.Proof of Concept
The
setApprovals()function code:While the Yield setApprovals() function doesn't use approve(0) for the Convex token, the corresponding Convex code does. Using approve(0) is comparable to use safeapprove(0) or safeDecreaseAllowance(0) if SafeERC20 was used.
Recommended Mitigation Steps
Use the same approve(0) call for convexToken that is used for curveToken before using approve(type(uint256).max) to avoid any edge cases such as the ERC20 approve race condition.