Skip to content

Commit

Permalink
update ECDSA version to fix a sig malleability issue
Browse files Browse the repository at this point in the history
  • Loading branch information
davidbrai committed Jul 18, 2023
1 parent 64a1974 commit fc33b6f
Show file tree
Hide file tree
Showing 3 changed files with 291 additions and 2 deletions.
235 changes: 235 additions & 0 deletions packages/nouns-contracts/contracts/external/openzeppelin/ECDSA.sol
@@ -0,0 +1,235 @@
// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (last updated v4.9.0) (utils/cryptography/ECDSA.sol)

pragma solidity ^0.8.0;

import { Strings } from '@openzeppelin/contracts/utils/Strings.sol';

/**
* @dev Elliptic Curve Digital Signature Algorithm (ECDSA) operations.
*
* These functions can be used to verify that a message was signed by the holder
* of the private keys of a given address.
*/
library ECDSA {
enum RecoverError {
NoError,
InvalidSignature,
InvalidSignatureLength,
InvalidSignatureS,
InvalidSignatureV // Deprecated in v4.8
}

function _throwError(RecoverError error) private pure {
if (error == RecoverError.NoError) {
return; // no error: do nothing
} else if (error == RecoverError.InvalidSignature) {
revert('ECDSA: invalid signature');
} else if (error == RecoverError.InvalidSignatureLength) {
revert('ECDSA: invalid signature length');
} else if (error == RecoverError.InvalidSignatureS) {
revert("ECDSA: invalid signature 's' value");
}
}

/**
* @dev Returns the address that signed a hashed message (`hash`) with
* `signature` or error string. This address can then be used for verification purposes.
*
* The `ecrecover` EVM opcode allows for malleable (non-unique) signatures:
* this function rejects them by requiring the `s` value to be in the lower
* half order, and the `v` value to be either 27 or 28.
*
* IMPORTANT: `hash` _must_ be the result of a hash operation for the
* verification to be secure: it is possible to craft signatures that
* recover to arbitrary addresses for non-hashed data. A safe way to ensure
* this is by receiving a hash of the original message (which may otherwise
* be too long), and then calling {toEthSignedMessageHash} on it.
*
* Documentation for signature generation:
* - with https://web3js.readthedocs.io/en/v1.3.4/web3-eth-accounts.html#sign[Web3.js]
* - with https://docs.ethers.io/v5/api/signer/#Signer-signMessage[ethers]
*
* _Available since v4.3._
*/
function tryRecover(bytes32 hash, bytes memory signature) internal pure returns (address, RecoverError) {
if (signature.length == 65) {
bytes32 r;
bytes32 s;
uint8 v;
// ecrecover takes the signature parameters, and the only way to get them
// currently is to use assembly.
/// @solidity memory-safe-assembly
assembly {
r := mload(add(signature, 0x20))
s := mload(add(signature, 0x40))
v := byte(0, mload(add(signature, 0x60)))
}
return tryRecover(hash, v, r, s);
} else {
return (address(0), RecoverError.InvalidSignatureLength);
}
}

/**
* @dev Returns the address that signed a hashed message (`hash`) with
* `signature`. This address can then be used for verification purposes.
*
* The `ecrecover` EVM opcode allows for malleable (non-unique) signatures:
* this function rejects them by requiring the `s` value to be in the lower
* half order, and the `v` value to be either 27 or 28.
*
* IMPORTANT: `hash` _must_ be the result of a hash operation for the
* verification to be secure: it is possible to craft signatures that
* recover to arbitrary addresses for non-hashed data. A safe way to ensure
* this is by receiving a hash of the original message (which may otherwise
* be too long), and then calling {toEthSignedMessageHash} on it.
*/
function recover(bytes32 hash, bytes memory signature) internal pure returns (address) {
(address recovered, RecoverError error) = tryRecover(hash, signature);
_throwError(error);
return recovered;
}

/**
* @dev Overload of {ECDSA-tryRecover} that receives the `r` and `vs` short-signature fields separately.
*
* See https://eips.ethereum.org/EIPS/eip-2098[EIP-2098 short signatures]
*
* _Available since v4.3._
*/
function tryRecover(
bytes32 hash,
bytes32 r,
bytes32 vs
) internal pure returns (address, RecoverError) {
bytes32 s = vs & bytes32(0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff);
uint8 v = uint8((uint256(vs) >> 255) + 27);
return tryRecover(hash, v, r, s);
}

/**
* @dev Overload of {ECDSA-recover} that receives the `r and `vs` short-signature fields separately.
*
* _Available since v4.2._
*/
function recover(
bytes32 hash,
bytes32 r,
bytes32 vs
) internal pure returns (address) {
(address recovered, RecoverError error) = tryRecover(hash, r, vs);
_throwError(error);
return recovered;
}

/**
* @dev Overload of {ECDSA-tryRecover} that receives the `v`,
* `r` and `s` signature fields separately.
*
* _Available since v4.3._
*/
function tryRecover(
bytes32 hash,
uint8 v,
bytes32 r,
bytes32 s
) internal pure returns (address, RecoverError) {
// EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature
// unique. Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines
// the valid range for s in (301): 0 < s < secp256k1n ÷ 2 + 1, and for v in (302): v ∈ {27, 28}. Most
// signatures from current libraries generate a unique signature with an s-value in the lower half order.
//
// If your library generates malleable signatures, such as s-values in the upper range, calculate a new s-value
// with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or
// vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept
// these malleable signatures as well.
if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) {
return (address(0), RecoverError.InvalidSignatureS);
}

// If the signature is valid (and not malleable), return the signer address
address signer = ecrecover(hash, v, r, s);
if (signer == address(0)) {
return (address(0), RecoverError.InvalidSignature);
}

return (signer, RecoverError.NoError);
}

/**
* @dev Overload of {ECDSA-recover} that receives the `v`,
* `r` and `s` signature fields separately.
*/
function recover(
bytes32 hash,
uint8 v,
bytes32 r,
bytes32 s
) internal pure returns (address) {
(address recovered, RecoverError error) = tryRecover(hash, v, r, s);
_throwError(error);
return recovered;
}

/**
* @dev Returns an Ethereum Signed Message, created from a `hash`. This
* produces hash corresponding to the one signed with the
* https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`]
* JSON-RPC method as part of EIP-191.
*
* See {recover}.
*/
function toEthSignedMessageHash(bytes32 hash) internal pure returns (bytes32 message) {
// 32 is the length in bytes of hash,
// enforced by the type signature above
/// @solidity memory-safe-assembly
assembly {
mstore(0x00, '\x19Ethereum Signed Message:\n32')
mstore(0x1c, hash)
message := keccak256(0x00, 0x3c)
}
}

/**
* @dev Returns an Ethereum Signed Message, created from `s`. This
* produces hash corresponding to the one signed with the
* https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`]
* JSON-RPC method as part of EIP-191.
*
* See {recover}.
*/
function toEthSignedMessageHash(bytes memory s) internal pure returns (bytes32) {
return keccak256(abi.encodePacked('\x19Ethereum Signed Message:\n', Strings.toString(s.length), s));
}

/**
* @dev Returns an Ethereum Signed Typed Data, created from a
* `domainSeparator` and a `structHash`. This produces hash corresponding
* to the one signed with the
* https://eips.ethereum.org/EIPS/eip-712[`eth_signTypedData`]
* JSON-RPC method as part of EIP-712.
*
* See {recover}.
*/
function toTypedDataHash(bytes32 domainSeparator, bytes32 structHash) internal pure returns (bytes32 data) {
/// @solidity memory-safe-assembly
assembly {
let ptr := mload(0x40)
mstore(ptr, '\x19\x01')
mstore(add(ptr, 0x02), domainSeparator)
mstore(add(ptr, 0x22), structHash)
data := keccak256(ptr, 0x42)
}
}

/**
* @dev Returns an Ethereum Signed Data with intended validator, created from a
* `validator` and `data` according to the version 0 of EIP-191.
*
* See {recover}.
*/
function toDataWithIntendedValidatorHash(address validator, bytes memory data) internal pure returns (bytes32) {
return keccak256(abi.encodePacked('\x19\x00', validator, data));
}
}
@@ -0,0 +1,54 @@
// SPDX-License-Identifier: MIT
// OpenZeppelin Contracts (last updated v4.9.0) (utils/cryptography/SignatureChecker.sol)

pragma solidity ^0.8.0;

import './ECDSA.sol';
import '@openzeppelin/contracts/interfaces/IERC1271.sol';

/**
* @dev Signature verification helper that can be used instead of `ECDSA.recover` to seamlessly support both ECDSA
* signatures from externally owned accounts (EOAs) as well as ERC1271 signatures from smart contract wallets like
* Argent and Gnosis Safe.
*
* _Available since v4.1._
*/
library SignatureChecker {
/**
* @dev Checks if a signature is valid for a given signer and data hash. If the signer is a smart contract, the
* signature is validated against that smart contract using ERC1271, otherwise it's validated using `ECDSA.recover`.
*
* NOTE: Unlike ECDSA signatures, contract signatures are revocable, and the outcome of this function can thus
* change through time. It could return true at block N and false at block N+1 (or the opposite).
*/
function isValidSignatureNow(
address signer,
bytes32 hash,
bytes memory signature
) internal view returns (bool) {
(address recovered, ECDSA.RecoverError error) = ECDSA.tryRecover(hash, signature);
return
(error == ECDSA.RecoverError.NoError && recovered == signer) ||
isValidERC1271SignatureNow(signer, hash, signature);
}

/**
* @dev Checks if a signature is valid for a given signer and data hash. The signature is validated
* against the signer smart contract using ERC1271.
*
* NOTE: Unlike ECDSA signatures, contract signatures are revocable, and the outcome of this function can thus
* change through time. It could return true at block N and false at block N+1 (or the opposite).
*/
function isValidERC1271SignatureNow(
address signer,
bytes32 hash,
bytes memory signature
) internal view returns (bool) {
(bool success, bytes memory result) = signer.staticcall(
abi.encodeWithSelector(IERC1271.isValidSignature.selector, hash, signature)
);
return (success &&
result.length >= 32 &&
abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector));
}
}
Expand Up @@ -20,8 +20,8 @@ pragma solidity ^0.8.19;
import './NounsDAOInterfaces.sol';
import { NounsDAOV3DynamicQuorum } from './NounsDAOV3DynamicQuorum.sol';
import { NounsDAOV3Fork } from './fork/NounsDAOV3Fork.sol';
import { SignatureChecker } from '@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol';
import { ECDSA } from '@openzeppelin/contracts/utils/cryptography/ECDSA.sol';
import { SignatureChecker } from '../external/openzeppelin/SignatureChecker.sol';
import { ECDSA } from '../external/openzeppelin/ECDSA.sol';
import { SafeCast } from '@openzeppelin/contracts/utils/math/SafeCast.sol';

library NounsDAOV3Proposals {
Expand Down

0 comments on commit fc33b6f

Please sign in to comment.