code-423n4 / 2022-04-badger-citadel-findings

0 stars 1 forks source link

Users wont be able to withdraw staked tokens since vesting function is called differently in interface and implementation #225

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadel.sol#L830 https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/interfaces/citadel/IVesting.sol#L5 https://github.com/code-423n4/2022-04-badger-citadel/blob/18f8c392b6fc303fe95602eba6303725023e53da/src/StakedCitadelVester.sol#L132

Vulnerability details

Impact

Users can't withdraw their funds

Proof of Concept

When withdrawing in staking the contract will send funds to vesting using setupVesting(). This is defined in the vesting interface. On the other hand, the vesting implementation calls this function vest() so setupVesting() isn't implemented.

All withdraws will revert. This doesn't come up in tests since the staking wasn't tested (at least in provided tests).

Recommended Mitigation Steps

Give the function the same name in interface and implementation

GalloDaSballo commented 2 years ago

Don't know if severity should be that high, but seems valid @dapp-whisperer wdyt?

jack-the-pug commented 2 years ago

Dup #9