code-423n4 / 2021-09-sushitrident-findings

0 stars 0 forks source link

HybridPool.sol lacks zero check for maserDeployer #10

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

hack3r-0m

Vulnerability details

https://github.com/sushiswap/trident/blob/9130b10efaf9c653d74dc7a65bde788ec4b354b5/contracts/pool/HybridPool.sol#L69-L71

Since the returned bool value (first parameter) of static call is ignored, there should be a check to make sure masterDeployer is non-zero and is a contract.

maxsam4 commented 3 years ago

masterDeployer is passed to the contract by the factory. It's not user input. Therefore, it is safe to assume that its not zero.

alcueca commented 3 years ago

It is safe to assume it is not zero in the current implementation, but not in any future implementations that reuse the HybridPool, but that implement a new factory, and the sponsor should be aware. To dispute, sponsor should show that there won't be future implementations, or that some process is in place to check inputs in governance actions or deployments.

alcueca commented 3 years ago

Or just document it in the smart contract