QA Report

[code423n4]

LOW:
1.
Title : ETH may be stuck forever

Impact :
In the https://code4rena.com/reports/2021-11-nested [M-08] the sponsored stated that this contract should not held any funds, and if there is any erc20 token accidentally send to this contract it is claimable by the owner through https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L132 unlockTokens function, since this contract has a receive https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L71 this contract can receive ETH, but there is no function to retrieve the ETH, therefore any ETH that was sent to this contract will be locked forever since the owner can't claim it also.

POC :
https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L71

Mitigation :
Add require to make this contract only receive etc from WETH address.

Title : missing validation between _inputTokenAmount and actual token that was inputed

Impact : The _transferInputTokens function is handling the token transfer from the user to this contract, however there is missing check whether the _inputTokenAmount is equal to the actual token that was transfer to this address, In the case AMP token, there is an external call when doing a transfer, an attacker can cause a mismatch when the token was transfer to this address, the attacker transfer the AMP token directly to this contract which inflating the balance of this address, therefore the difference between the _inputTokenAmount and the actual balance that this contract has, would be massive.

POC :

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

Title : There is no limit on how many operator that can be added

Impact : In the https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L100 the owner can add an operator, however there is no limit on how many operator can be added by the owner, if the operator inflated to a big value, the removeOperator would be failing on run out of gas, therefore the operator would be inflated and can't be removed.

POC :
https://github.com/code-423n4/2022-02-nested/blob/main/contracts/NestedFactory.sol#L100

[maximebrugel]

« ETH may be stuck forever » (Duplicated)

#44

« Missing validation between _inputTokenAmount and actual token that was inputed » (Disputed)

We don’t want to revert if the token has fees on transfers.

« There is no limit on how many operator that can be added » (Acknowledged)

[harleythedogC4]

The first point is better suited as a duplicate of #37 (low severity), since both do not mention that anyone can actually use the stuck eth to their advantage (which #44 describes). The third point is a valid observation but unlikely to cause any issues since the owner is a multisig behind a timelock.

[harleythedogC4]

My personal judgements:

1. "Title : ETH may be stuck forever". Valid and low-critical.
2. "Title : missing validation between _inputTokenAmount and actual token that was inputed". The whole point of using the delta of the balance is to get the exact amount of tokens transferred in and not trust that it is equal to _inputTokenAmount. Invalid.
3. "Title : There is no limit on how many operator that can be added". Valid and low-critical.

[harleythedogC4]

Now, here is the methodology I used for calculating a score for each QA report. I first assigned each submission to be either non-critical (1 point), very-low-critical (5 points) or low-critical (10 points), depending on how severe/useful the issue is. The score of a QA report is the sum of these points, divided by the maximum number of points achieved by a QA report. This maximum number was 26 points, achieved by #66.

The number of points achieved by this report is 20 points.
Thus the final score of this QA report is (20/26)*100 = 77.