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

1 stars 0 forks source link

unwrap() can be called by anybody leading to loss of funds. #87

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

GeekyLumberjack

Vulnerability details

Impact

The caller of unwrap() would receive all of the unwrapped convex tokens. Potentially depriving the user of all collateral and any rewards.

Proof of Concept

This portion of the readme describes the process that leads to the vulnerability.

To repay a debt and withdraw Convex collateral, unstaking it in the process, the user will execute this batch through the Ladle:

1. Approve the Ladle to move the debt tokens (permit)
2. Call user_checkpoint to update the user rewards before any transfer that would erase them (route)
3. Move the debt tokens to the debt token contract (transfer)
4. Update accounting, burn debt tokens and transfer out collateral to the ConvexYieldWrapper (pour)
5. Unwrap the wrappedConvex and send the resulting convex to the user. This checkpoints rewards. (route)
6. If desired, claim rewards from the ConvexYieldWrapper (route)

When step 4 is executed the funds will be in ConvexYieldWrapper and an attacker who calls unwrap() can have all the funds and rewards sent to any address.

Tools Used

Manual analysis

Recommended Mitigation Steps

Add access controls to unwrap()

iamsahu commented 2 years ago

As specified in the readme: https://github.com/code-423n4/2022-01-yield#interacting-directly-with-smart-contracts

GalloDaSballo commented 2 years ago

The finding is invalid as the contract is explicitly not supposed to hold any funds as it is intended to be interacted with by other contracts