Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add overloaded checkNSignatures to pass in the msg.sender #557

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions contracts/Safe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -259,19 +259,19 @@ contract Safe is
uint256 _threshold = threshold;
// Check that a threshold is set
require(_threshold > 0, "GS001");
checkNSignatures(dataHash, data, signatures, _threshold);
checkNSignatures(msg.sender, dataHash, data, signatures, _threshold);
}

/**
* @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise.
* @notice Checks whether the signature provided is valid for the provided data, hash and executor. Reverts otherwise.
* @dev Since the EIP-1271 does an external call, be mindful of reentrancy attacks.
* @param dataHash Hash of the data (could be either a message hash or transaction hash)
* @param data That should be signed (this is passed to an external validator contract)
* @param signatures Signature data that should be verified.
* Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash.
* @param requiredSignatures Amount of required valid signatures.
*/
function checkNSignatures(bytes32 dataHash, bytes memory data, bytes memory signatures, uint256 requiredSignatures) public view {
function checkNSignatures(address executor, bytes32 dataHash, bytes memory data, bytes memory signatures, uint256 requiredSignatures) public view {
// Check that the provided signature data is not too short
require(signatures.length >= requiredSignatures.mul(65), "GS020");
// There cannot be an owner with address 0.
Expand Down Expand Up @@ -318,7 +318,7 @@ contract Safe is
// When handling approved hashes the address of the approver is encoded into r
currentOwner = address(uint160(uint256(r)));
// Hashes are automatically approved by the sender of the message or when they have been pre-approved via a separate transaction
require(msg.sender == currentOwner || approvedHashes[currentOwner][dataHash] != 0, "GS025");
require(executor == currentOwner || approvedHashes[currentOwner][dataHash] != 0, "GS025");
} else if (v > 30) {
// If v > 30 then default va (27,28) has been adjusted for eth_sign flow
// To support eth_sign and similar we adjust v and hash the messageHash with the Ethereum message prefix before applying ecrecover
Expand All @@ -333,6 +333,19 @@ contract Safe is
}
}

/**
* @notice Checks whether the signature provided is valid for the provided data and hash. Reverts otherwise.
* @dev Since the EIP-1271 does an external call, be mindful of reentrancy attacks.
* @param dataHash Hash of the data (could be either a message hash or transaction hash)
* @param data That should be signed (this is passed to an external validator contract)
* @param signatures Signature data that should be verified.
* Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash.
* @param requiredSignatures Amount of required valid signatures.
*/
function checkNSignatures(bytes32 dataHash, bytes memory data, bytes memory signatures, uint256 requiredSignatures) public view {
return checkNSignatures(msg.sender, dataHash, data, signatures, requiredSignatures);
}

/**
* @notice Marks hash `hashToApprove` as approved.
* @dev This can be used with a pre-approved hash transaction signature.
Expand Down