L01 - NativeMetaTransaction::verify - ecrecover is prone to signature malleability and should not be used for signature verification

Link: https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/meta-transaction/NativeMetaTransaction.sol#L98

Using ecrecover for signature verification has potential security issues (signature malleability). Also, it does not provide input validation, which can lead to unexpected behavior if the input data is malformed.

Recommendations:

Use OpenZeppelin's ECDSA library.

L02 - MembershipFactory::updateMembershipImplementation - avoid single step ownership transfer

Link: https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L206

Updating crucial contract addresses (like, the address of the MembershipERC1155 implementation contract) in a single step can be risky because it introduces the potential for mistakes or malicious attacks that could compromise the contract’s functionality or security.

Recommendations:

Use a two-step process to update the address:

• Propose an address update and save it as a pending change

• After some delay, which provides time to review and verify the new address, the change can be finalized

L03 - MembershipFactory::createNewDAOMembership - TierConfig.power is not used

Links:

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/libraries/MembershipDAOStructs.sol#L34
https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/tokens/MembershipERC1155.sol#L169
https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L55

Currently the power is hardcoded to be 2. This is used in the shareOf function and no matter what the DAO creator provides for the power values of the various tiers, this won't have any influence on the share calculation.

Recommendations:

Either delete the power value from the TierConfig struct as this is misleading and can lead to wrong assumptions by the DAO creator. Or, update the code so that it uses the provided power value. If the power value is used it should be validated together with the provided price value and the shareOf function needs to be updated accordingly.