code-423n4 / 2024-10-kleidi-findings

1 stars 0 forks source link

QA Report #60

Open howlbot-integration[bot] opened 3 weeks ago

howlbot-integration[bot] commented 3 weeks ago

See the markdown file with the details of this report here.

c4-judge commented 2 weeks ago

GalloDaSballo marked the issue as grade-b

GalloDaSballo commented 1 week ago

Finding Heading TODO calculateAddress() function is vulnerable to frontrunning I Safe calling itself with empty calldata is potentially dangerous if a fallback handler is configured I Insufficient validation for owners in RecoverySpellFactory.sol L Consider restricting unbounded loops to protect against user error I Bad proposal timestamp cleanup leads to inconsistent behavior -3 Q-01 Arbitrum and Optimism chainIds missing in SystemDeploy.s.sol L Q-02 InstanceDeployer immutable variable verification missing in SystemDeploy.s.sol I Q-03 Consider making RecoverySpell deployment permissioned I Q-04 Wrong comment in ConfigurablePause.sol NC Q-05 Unused Code I Q-06 Unnecessary salt in InstanceDeployer.sol I, wouldn't make the change over the recommendation due to lack of backing Q-07 Reachable assert() functions in InstanceDeployer.sol will consume all of the remaining gas I Q-08 s parameter in signature can be non-zero due to corrupted memory I Q-09 Rename newRecoveryThreshold to recoveryThreshold I Q-10 Add recovery spell owners to the recovery spell signature TODO Q-11 Comment that safe owners can call executeWhitelistedBatch() is wrong TODO Q-12 Comment that safe owners can execute whitelisted calldatas is wrong TODO Q-13 Comment that require() can not be reached is wrong TODO Q-14 indexes variable is redundant TODO Q-15 Delete _calldataList is redundant TODO Q-16 _setPauseTime() in setGuardian() is redundant TODO Q-17 checkCalldata() does not support calls with zero calldata TODO Q-18 Index copying logic can be shortcut TODO

Stopping here, overall I think it would have been best to handpick half of these findings and add more info and context

TODO - 10 I - 9 L - 2 -3 - 1 NC - 1 I, wouldn't make the change over the recommendation due to lack of backing - 1

2L 1NC -3

thebrittfactor commented 1 week ago

For awarding purposes, C4 staff have marked as tied for 2nd place.