code423n4 commented 3 years ago

Handle

loop

Vulnerability details

The constructor of PoolFactory takes the argument _feeReceiver, which is used to set the state variable feeReceiver. There does not seem to be a reason to initiate this as the zero address since LeveragedPools require feeReceiver to be set to a non-zero address in order to be deployed using the deployPool() function.

Impact

No big impact considering feeReceiver can still be set or changed using the setFeeReceiver() function.

Proof of Concept

PoolFactory constructor: https://github.com/tracer-protocol/perpetual-pools-contracts/blob/646360b0549962352fe0c3f5b214ff8b5f73ba51/contracts/implementation/PoolFactory.sol#L48

Tools Used

Slither

Recommended Mitigation Steps

Add require(_feeReceiver != address(0), "address cannot be null"); to avoid needing an extra function call if PoolFactory gets initiated with the zero address.

kumar-ish commented 3 years ago

Duplicate of #11

GalloDaSballo commented 3 years ago