Skip to content

Spearbit #40: Fixed a potential signature replay vulnerability #804

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 5, 2024
Merged

Spearbit #40: Fixed a potential signature replay vulnerability #804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 21 additions & 20 deletions contracts/src/external/Hyperdrive.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,6 @@ abstract contract Hyperdrive is
"PermitForAll(address owner,address spender,bool _approved,uint256 nonce,uint256 deadline)"
);

/// @notice This contract's EIP712 domain separator.
bytes32 public immutable DOMAIN_SEPARATOR; // solhint-disable-line var-name-mixedcase

/// @notice Instantiates a Hyperdrive pool.
/// @param _config The configuration of the pool.
/// @param _target0 The target0 address.
Expand All @@ -118,22 +115,6 @@ abstract contract Hyperdrive is
target2 = _target2;
target3 = _target3;
target4 = _target4;

// NOTE: It's convenient to keep this in the `Hyperdrive.sol`
// entry-point to avoiding issues with initializing the domain
// separator with the contract address. If this is moved to one of
// the targets, the domain separator will need to be computed
// differently.
DOMAIN_SEPARATOR = keccak256(
abi.encode(
keccak256(
"EIP712Domain(string version,uint256 chainId,address verifyingContract)"
),
keccak256(bytes("1")),
block.chainid,
address(this)
)
);
}

/// @notice If we get to the fallback function, we make a read-only
Expand Down Expand Up @@ -354,7 +335,7 @@ abstract contract Hyperdrive is
abi.encodeCall(
HyperdriveTarget0.permitForAll,
(
DOMAIN_SEPARATOR,
domainSeparator(),
PERMIT_TYPEHASH,
owner,
spender,
Expand All @@ -376,6 +357,26 @@ abstract contract Hyperdrive is
}
}

/// EIP712

/// @notice Computes the EIP712 domain separator which prevents user signed
/// messages for this contract to be replayed in other contracts:
/// https://eips.ethereum.org/EIPS/eip-712.
/// @return The EIP712 domain separator.
function domainSeparator() public view returns (bytes32) {
return
keccak256(
abi.encode(
keccak256(
"EIP712Domain(string version,uint256 chainId,address verifyingContract)"
),
keccak256(bytes("1")),
block.chainid,
address(this)
)
);
}

/// Helpers ///

/// @dev Makes a delegatecall to the extras contract with the provided
Expand Down
3 changes: 1 addition & 2 deletions contracts/src/interfaces/IERC20Forwarder.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ interface IERC20Forwarder is IERC20 {

/// @notice The EIP712 domain separator for this contract.
/// @return The domain separator.
// solhint-disable-next-line func-name-mixedcase
function DOMAIN_SEPARATOR() external view returns (bytes32);
function domainSeparator() external view returns (bytes32);

/// @notice The EIP712 typehash for the permit struct used by this contract.
/// @return The permit typehash.
Expand Down
3 changes: 1 addition & 2 deletions contracts/src/interfaces/IMultiTokenMetadata.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,5 @@ interface IMultiTokenMetadata {

/// @notice Gets the EIP712 domain separator of the MultiToken.
/// @return The EIP712 domain separator of the MultiToken.
// solhint-disable func-name-mixedcase
function DOMAIN_SEPARATOR() external view returns (bytes32);
function domainSeparator() external view returns (bytes32);
}
36 changes: 18 additions & 18 deletions contracts/src/token/ERC20Forwarder.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ contract ERC20Forwarder is IERC20Forwarder {
/// @notice A mapping from a user to their nonce for permit signatures.
mapping(address user => uint256 nonce) public nonces;

/// @notice The EIP712 domain separator for this contract
bytes32 public immutable DOMAIN_SEPARATOR; // solhint-disable-line var-name-mixedcase

/// @notice The EIP712 typehash for the permit struct used by this contract
/// to validate permit signatures.
bytes32 public constant PERMIT_TYPEHASH =
Expand All @@ -47,21 +44,24 @@ contract ERC20Forwarder is IERC20Forwarder {

// Load the initialization data from the factory.
(token, tokenId) = factory.getDeployDetails();
}

// Computes the EIP712 domain separator which prevents user signed
// messages for this contract to be replayed in other contracts:
// https://eips.ethereum.org/EIPS/eip-712.
DOMAIN_SEPARATOR = keccak256(
abi.encode(
keccak256(
"EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
),
keccak256(bytes(token.name(tokenId))),
keccak256(bytes("1")),
block.chainid,
address(this)
)
);
/// @notice Computes the EIP712 domain separator which prevents user signed
/// messages for this contract to be replayed in other contracts:
/// https://eips.ethereum.org/EIPS/eip-712.
/// @return The EIP712 domain separator.
function domainSeparator() public view returns (bytes32) {
return
keccak256(
abi.encode(
keccak256(
"EIP712Domain(string version,uint256 chainId,address verifyingContract)"
),
keccak256(bytes("1")),
block.chainid,
address(this)
)
);
}

/// @notice Returns the decimals for this ERC20 interface. Hyperdrive's
Expand Down Expand Up @@ -229,7 +229,7 @@ contract ERC20Forwarder is IERC20Forwarder {
bytes32 structHash = keccak256(
abi.encodePacked(
"\x19\x01",
DOMAIN_SEPARATOR,
domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down
6 changes: 3 additions & 3 deletions contracts/test/MockMultiToken.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ contract MockMultiToken is HyperdriveMultiToken, MockHyperdriveBase {
);

/// @notice This contract's EIP712 domain separator.
bytes32 public immutable DOMAIN_SEPARATOR; // solhint-disable-line var-name-mixedcase
bytes32 public immutable domainSeparator; // solhint-disable-line var-name-mixedcase

constructor(
bytes32 _linkerCodeHash,
Expand Down Expand Up @@ -106,7 +106,7 @@ contract MockMultiToken is HyperdriveMultiToken, MockHyperdriveBase {
// separator with the contract address. If this is moved to one of
// the targets, the domain separator will need to be computed
// differently.
DOMAIN_SEPARATOR = keccak256(
domainSeparator = keccak256(
abi.encode(
keccak256(
"EIP712Domain(string version,uint256 chainId,address verifyingContract)"
Expand Down Expand Up @@ -189,7 +189,7 @@ contract MockMultiToken is HyperdriveMultiToken, MockHyperdriveBase {
abi.encodeCall(
HyperdriveTarget0.permitForAll,
(
DOMAIN_SEPARATOR,
domainSeparator,
PERMIT_TYPEHASH,
owner,
spender,
Expand Down
16 changes: 8 additions & 8 deletions test/units/ERC20Forwarder.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ contract ERC20ERC20ForwarderFactoryTest is BaseTest {
keccak256(
abi.encodePacked(
"\x19\x01",
forwarder.DOMAIN_SEPARATOR(),
forwarder.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -162,7 +162,7 @@ contract ERC20ERC20ForwarderFactoryTest is BaseTest {
keccak256(
abi.encodePacked(
"\x19\x01",
forwarder.DOMAIN_SEPARATOR(),
forwarder.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -199,7 +199,7 @@ contract ERC20ERC20ForwarderFactoryTest is BaseTest {
keccak256(
abi.encodePacked(
"\x19\x01",
forwarder.DOMAIN_SEPARATOR(),
forwarder.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -229,7 +229,7 @@ contract ERC20ERC20ForwarderFactoryTest is BaseTest {
keccak256(
abi.encodePacked(
"\x19\x01",
forwarder.DOMAIN_SEPARATOR(),
forwarder.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -276,7 +276,7 @@ contract ERC20ERC20ForwarderFactoryTest is BaseTest {
keccak256(
abi.encodePacked(
"\x19\x01",
forwarder.DOMAIN_SEPARATOR(),
forwarder.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -316,7 +316,7 @@ contract ERC20ERC20ForwarderFactoryTest is BaseTest {
keccak256(
abi.encodePacked(
"\x19\x01",
forwarder.DOMAIN_SEPARATOR(),
forwarder.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -357,7 +357,7 @@ contract ERC20ERC20ForwarderFactoryTest is BaseTest {
keccak256(
abi.encodePacked(
"\x19\x01",
forwarder.DOMAIN_SEPARATOR(),
forwarder.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -407,7 +407,7 @@ contract ERC20ERC20ForwarderFactoryTest is BaseTest {
bytes32 structHash = keccak256(
abi.encodePacked(
"\x19\x01",
multiToken.DOMAIN_SEPARATOR(),
multiToken.domainSeparator(),
keccak256(
abi.encode(
MULTITOKEN_PERMIT_TYPEHASH,
Expand Down
10 changes: 5 additions & 5 deletions test/units/MultiToken.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ contract MultiTokenTest is BaseTest {
bytes32 structHash = keccak256(
abi.encodePacked(
"\x19\x01",
multiToken.DOMAIN_SEPARATOR(),
multiToken.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -115,7 +115,7 @@ contract MultiTokenTest is BaseTest {
bytes32 structHash = keccak256(
abi.encodePacked(
"\x19\x01",
multiToken.DOMAIN_SEPARATOR(),
multiToken.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -156,7 +156,7 @@ contract MultiTokenTest is BaseTest {
bytes32 structHash = keccak256(
abi.encodePacked(
"\x19\x01",
multiToken.DOMAIN_SEPARATOR(),
multiToken.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -197,7 +197,7 @@ contract MultiTokenTest is BaseTest {
bytes32 structHash = keccak256(
abi.encodePacked(
"\x19\x01",
multiToken.DOMAIN_SEPARATOR(),
multiToken.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down Expand Up @@ -277,7 +277,7 @@ contract MultiTokenTest is BaseTest {
bytes32 structHash = keccak256(
abi.encodePacked(
"\x19\x01",
multiToken.DOMAIN_SEPARATOR(),
multiToken.domainSeparator(),
keccak256(
abi.encode(
PERMIT_TYPEHASH,
Expand Down