search

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

0 stars 1 forks source link

Impractical Entry/Exit fees are allowed #35

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-nested/blob/main/contracts/NestedFactory.sol#L159

Vulnerability details

Impact

It seems that Owner is allowed to set entry/exit fees to be 100% of amount. An entry fees of 100% will be impractical and will lead to all order amount to be gone in fees at NestedFactory.sol#L378

Proof of Concept

  1. Admin call setEntryFees function and set _entryFees as 10000

  2. entryFees becomes 100%

  3. Now assume an order is submitted via _submitInOrders

  4. Entry fees will be deducted from amount spent

  5. Since fees is 100% so feesDeducted=amountSpent which means amountSpent effectively becomes 0 as all of it went for fees

Recommended Mitigation Steps

Decide a max percentage of fees say 10% which can be charged and then change the require condition accordingly. Same logic need to be applied for exit fees

require(_entryFees <= MAX_PERCENTAGE, "NF: FEES_OVERFLOW");
obatirou commented 2 years ago

Disputed

It's about warden 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

best practice/suggestion, will downgrade to QA