code-423n4 / 2022-12-prepo-findings

0 stars 1 forks source link

Deposit record does not update properly for withdrawals #294

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol#L73 https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/WithdrawHook.sol#L73 https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/DepositRecord.sol#L35-L38

Vulnerability details

Impact

A side effect of calling deposit in the Collateral contract is that the userToDeposits map in the DepositRecord contract is updated. However, when the user withdraws funds, this userToDeposits map is not updated to reflect their total deposited amount.

The impact of this is that when the user goes to perform a deposit again in the future, the recordDeposit function might revert incorrectly since the amount in userToDeposits[_sender] is incorrect. This will cause the user to be unable to create a new deposit

Tools Used

Manual inspection

Recommended Mitigation Steps

Add functionality to update the userToDeposits state variable when withdrawals occur

hansfriese commented 1 year ago

I think it's an intended behavior from this comment

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid

Picodes commented 1 year ago

Indeed it seems to be the intended behavior

c4-judge commented 1 year ago

Picodes marked the issue as primary issue