coinbase / smart-wallet

MIT License
258 stars 47 forks source link

Change implementation owner to address(1) from address(0) #44

Closed stevieraykatz closed 3 months ago

stevieraykatz commented 3 months ago

This Code4rena finding correctly points out that the implementation being owned by address(0) might expose attack surface area. Though we cannot find a way to exploit this currently, it's trivial to change the implementations owner to address(1) to completely eliminate this vector.

stevieraykatz commented 3 months ago

Closing out -- SignatureCheckerLib.isValidSignatureNow appropriately returns false for address(0):

                    // `returndatasize()` will be `0x20` upon success, and `0x00` otherwise.
                    if iszero(or(iszero(returndatasize()), xor(signer, mload(t)))) {
                        isValid := 1
                        mstore(0x60, 0) // Restore the zero slot.
                        mstore(0x40, m) // Restore the free memory pointer.
                        break
                    }

Unit test