QA Report #285
Labels
bug
Something isn't working
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
1. Excess ether sent to FixedPricePassThruGate is lost (low)
passThruGate() redirects to a
beneficiary
onlygate.ethCost
, requiring thatmsg.value >= gate.ethCost
. As there are no other ways to access native tokens held by this contract, the cumulative excess value, a sum ofmsg.value - gate.ethCost
, will be permanently frozen within the contract.Proof of Concept
Only
gate.ethCost
is now forwarded:https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/FixedPricePassThruGate.sol#L53-L53
Recommended Mitigation Steps
Pass-though the whole msg.value as the excess is not used:
2. One step owner change (non-critical)
One step process offers no protection for the cases when ownership transfer is performed mistakenly or with any malicious intent.
Adding a modest complexity of an additional step and a delay is a low price to pay for having time to evaluate the change.
Proof of Concept
https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/VoterID.sol#L148-L155
https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleIdentity.sol#L57-L62
Recommended Mitigation Steps
Consider utilizing two-step owner rights transferring process (proposition and acceptance in the separate actions) with a noticeable delay between the steps to enforce the transparency and stability of the system.
3. No zero address checks in constructors (non-critical)
Functionality of the system will be unavailable if core config addresses be set to zero.
Proof of Concept
https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleIdentity.sol#L50-L55
https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/MerkleEligibility.sol#L33-L37
https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/VoterID.sol#L102-L114
Recommended Mitigation Steps
Consider adding zero address checks as operation mistakes mitigation tends to out-weight gas optimization in such core config one time setups.
The text was updated successfully, but these errors were encountered: