search

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

1 stars 0 forks source link

Extend [ Possible logic missmatch ] #45

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

0x1f8b

Vulnerability details

This is an extension of a previous issue

Proof of Concept

removeVault in ConvexYieldWrapper doesn't throw any error with account=address(0) so user wont be noticie if he use:

alcueca commented 2 years ago

It is a weird use case to build a vault, add it to the wrapper, then remove it from the wrapper in the same transaction. However, it is a valid transaction, and if we ever need it, it would be disconcerting not to do it.

GalloDaSballo commented 2 years ago

@alcueca can you please confirm whether you implemented the suggestion or not?

alcueca commented 2 years ago

When implementing a fix, we realized that the finding is not valid.

addVault(0) will revert because the vault needs to exist in the Cauldron.

removeVault(0, 0x0000) will revert because the owner of a non-existing vault is address(0). removeVault(0, 0x1234) will revert because 0x1234 would have needed to add vault 0 first.

GalloDaSballo commented 2 years ago

Per the sponsor comment calling with 0 as input will cause a revert at the cauldron level.

In lack of a POC, am marking the finding invalid