code-423n4 / 2023-10-brahma-findings

8 stars 7 forks source link

Don't Follow ERC1271 Standard #199

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/safe-global/safe-contracts/blob/main/contracts/interfaces/ISignatureValidator.sol#L7 https://github.com/safe-global/safe-contracts/blob/main/contracts/interfaces/ISignatureValidator.sol#L20 https://github.com/MolochVentures/moloch/blob/master/v1_contracts/gnosis-safe/GnosisSafe.sol#L185

Vulnerability details

Impact

isValidSignature() will return wrong value

Proof of Concept

As Per EIP-1271 standard ERC1271_MAGIC_VAULE should be 0x1626ba7e instead of 0x20c13b0b and function name should be isValidSignature(bytes32, bytes) instead of isValidSignature(bytes, bytes). Due to this, the signature verifier contract goes fallback function and returns unexpected value and never return ERC1271_MAGIC_VALUE and always reverts the execTransaction function.

related report : https://github.com/code-423n4/2023-01-biconomy-findings/issues/288

Tools Used

manual code review vscode solidityScan

Recommended Mitigation Steps

Follow EIP-1271 standard.

Assessed type

Other

c4-pre-sort commented 11 months ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #46

c4-judge commented 11 months ago

alex-ppg marked the issue as unsatisfactory: Invalid