code-423n4 / 2022-01-yield-findings

1 stars 0 forks source link

Unsafe approve #52

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0x1f8b

Vulnerability details

Impact

Unsafe approve.

Proof of Concept

The method setApprovals inside the contract ConvexStakingWrapper doesn't check the result of the approve call. ERC20 standard specify that the token can return false if the approve was not made, so it's mandatory to check the result of approve methods.

Tools Used

Manual review.

Recommended Mitigation Steps

Use safeApprove

alcueca commented 2 years ago

Only that we are calling contracts (curve, convex) for which the source code is known, and which have no conditions in which the approve can't be made.

The recommendation would be valid if the contracts we call approve on would be user-supplied, but not when they are governance-supplied.

GalloDaSballo commented 2 years ago

The sponsor has researched the specific token used by the contract and verified that there's no need to check it's return value.

I've also gone through the CVX source and can confirm that the token will always return true or revert.

For this reason the finding is invalid