PanopticFactory
can be bricked and become unusable
#523
Labels
bug
Something isn't working
downgraded by judge
Judge downgraded the risk level of this issue
grade-a
primary issue
Highest quality submission among a set of duplicates
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
🤖_16_group
AI based duplicate group recommendation
sponsor disputed
Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Lines of code
https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticFactory.sol#L134
https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticFactory.sol#L222-L224
Vulnerability details
Impact
It is possible to brick
PanopticFactory
when it is deployed in a "permissionless" way if the factory was left unitialized. This issue could stay unnoticed for a while as the factory will work fine if left unitialized, and the owner set to address zero is a valid use case.Given that they are now the owners no one but them will be able to call
deployNewPool
, impacting the availability of the protocol for other users. They will also monopolize thedonorNFT
as they will be the only user able to invokeissueNFT
if they deploy a new pool.Severity Considerations
I personally consider frontrunning the initialize function as low severity, as the protocol can simply deploy a new contract. However, this is not the issue I'm describing here.
The problem is that the factory will work fine even if left uninitialized and an escalation can occur after some time has already passed and users have already started using this contract. Given the implications regarding the NFTs issued and the pool tracking inside the factory, I consider this role escalation to be of Medium severity.
Proof of Concept
A zero-address owner is a valid use case as the factory can be used in a permissionless way when calling
deployNewPool
:https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticFactory.sol#L222-L224
By default, the owner will be the zero address unless
initialize
is called. Suppose that the function isn't called after deployment, and it goes unnoticed if the intention is to deploy a permissionless factory; it will work fine, and users will start using it and deploying new pools, while the factory will continue to issuedonorNFT
s and keep track of each pool ins_getPanopticPool
.After a while, an attacker may realize that the factory was never initialized. At that point, they can simply call
initialize
to take ownership of the contract:https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticFactory.sol#L134
Tools Used
Manual review
Recommended Mitigation Steps
Consider making
deployNewPool
fail ifs_initialized
isfalse
. This way, the correct method to initialize the contract would be to callinitialize
with the zero address if the intention is to use a permissionless factory, preventing a takeover of the contract after the users start using it.Assessed type
Invalid Validation
The text was updated successfully, but these errors were encountered: