search

code-423n4 / 2022-06-nibbl-findings

1 stars 0 forks source link

Missing notBoughtOut modifier #313

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/NibblVault.sol#L454

Vulnerability details

Impact

On the withdrawUnsettledBids function, unsettledBid can be withdraw when buyout is rejected. However, the contract is missing notBoughtOut modifier in the function. From that reason, during buyout period the function can be called. On the redeem function, It is directly getting value from totalUnsettledBids and maybe can leads to send to user more ETH than excepted.

Proof of Concept

https://github.com/code-423n4/2022-06-nibbl/blob/main/contracts/NibblVault.sol#L454

Tools Used

Code Review

Recommended Mitigation Steps

Consider implementing related modifier.

KenzoAgada commented 2 years ago

Hmm... I might be missing something, but... During buyout process, the totalUnsettledBids variable has not taken the current buyout bid into account yet. So the accounting is correct and all the unsettled bids are accounted for in both totalUnsettledBids and the contract balance. Therefore, if somebody withdraws his bid during the buyout process, both totalUnsettledBids and the contract balance will be updated, and afterwards redeem will work as expected. So unless I am missing something, issue seems invalid.

mundhrakeshav commented 2 years ago

totalUnsettledBids is updated in rejectBuyout

HardlyDifficult commented 2 years ago

Thanks for the comments above.

As far as I can tell there is no issue here. For submissions like this in the future, please consider including a POC - it helps when trying to confirm our understanding of your report.