QA Report

[code423n4]

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

[c4-judge]

0xean marked the issue as grade-a

[c4-judge]

0xean marked the issue as selected for report

[novaknole20]

Because it is a generic executor we don't check for this and it is up to the user. [01]

It is ok to send data in a tx even when it gets send to an EOA wallet. See this TX from euler to their recent attacker https://etherscan.io/tx/0x8f2b61a0c70012df1e3d918e7ac6486a1c332a9e530f3d4061735c6620f960d9 [02]

[03] is the same as [02]

Frontrunning this function call implies that the frontrunner has permission to call this function which is already a security risk in itself [04]

That is not fully right. _grantWithConditiion also gets called from _grant with the ALLOW_FLAG as the condition. So the check is there and everything is as described in the documentation [05]

We didn't do it because it increases the gas cost for no beneficial reason [06]

We don't neeed the deposit functions for these token standards because they have a callback when the user uses the safe... functions and thus we get the events we need for our subgraph [07]

Think of applySingleTragetPermissions to be the simpler form to grant permissions and also the more cost effective (less gas used) [08]

Signature validator and trusted forwarder can be set to 0 to disable the functionatliy. DAO initialize only gets called from DAOFactory and there we use the DAOFactory as the initial owner. So no need for the check [09]

We don't mint NFTs in batches and don't have the functionality to do so. [11]

We decided to used 0.8.17. No need to upgrade and retest everything for no benefit [12]

No need to use a constant for a value that is only used once [14]

[c4-sponsor]

novaknole20 marked the issue as sponsor confirmed

[0xean]

01 - Low Risk 02 - Low Risk 03 - Low Risk 04 - NC 05 - NC 06 - NC 07 - NC 08 - NC 09 - NC - already declared in known issues from C4udit 10 - NC 11 - Low Risk 12 - NC 13 - Low Risk 14 - NC 15 - NC 16 - NC 17 - NC 18 - NC

[0xean]

11 -this is more of a general suggestion, should be NC