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

0 stars 1 forks source link

`destroy` function doesn't check if `exitFees` is set #19

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nested/blob/b4a153c943d54755711a2f7b80cbbf3a5bb49d76/contracts/NestedFactory.sol#L243-L275

Vulnerability details

Impact

This issue might lead to 0 amountFees

Proof of Concept

The destroy function does not check if exitFees is set, therefore in a scenario wherein the owner does not set the value of exitFees, its default value would be 0. In line 264 we can see: uint256 amountFees = (amountBought * exitFees) / 10000; and this means that if the exitFees == 0 then the amountFees would evaluate to 0 as well.

Tools Used

Manual analysis

Recommended Mitigation Steps

I recommend adding a require statement require(exitFees != 0);

obatirou commented 2 years ago

Disputed

It's about wardens appreciation of our ownership architecture versus ours. We can imagine many other malicious scenarios, assuming that the Multisig/Timelock/OwnerProxy combination is not enough to prevent the protocol from being compromised.

jack-the-pug commented 2 years ago

Why cant it be no exitFee? It's a feature, not a bug.