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

Multichain signature reuse / replay risk in HolographERC20.sol #173

Closed
code423n4 opened this issue Oct 24, 2022 · 1 comment
Closed

Multichain signature reuse / replay risk in HolographERC20.sol #173

code423n4 opened this issue Oct 24, 2022 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate This issue or pull request already exists invalid This doesn't seem right

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/HolographERC20.sol#L460
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/HolographERC20.sol#L470
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/HolographERC20.sol#L481

Vulnerability details

Impact

User is able to copy user's signature from chain A to grand malicious approval in Chain B and malicious actor can steal user's fund in chain B.

The HolographERC20.sol is likely to be deployed cross-chain. According to the documentation.

https://docs.holograph.xyz/holograph-protocol/protocol-specification

A deployment object is created that includes certain preliminary configurations. This object is hashed and signed by the deploying wallet, and the configuration parameters are passed along to a Factory smart contract on the initially deployed blockchain. The Factory smart contract uses the configuration and original signer as the salt in the CREATE2 function. The signed configuration ensures that a deployment cannot be modified or changed, without altering the deployment contract address. This allows for the deployment of an identical smart contract on any other EVM blockchain by any wallet or contract.

In this case, the signature schema has security risk

bytes32 structHash = keccak256(
  abi.encode(
	0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9,
	account,
	spender,
	amount,
	_useNonce(account),
	deadline
  )
);
bytes32 hash = _hashTypedDataV4(structHash);
address signer = ECDSA.recover(hash, v, r, s);
require(signer == account, "ERC20: invalid signature");
if (_isEventRegistered(HolographERC20Event.beforeApprove)) {
  require(SourceERC20().beforeApprove(account, spender, amount));
}

because the chainId is missing, User is able to copy user's signature from chain A to grand malicious approval in Chain B and malicious actor can steal user's fund in chain B.

Proof of Concept

  1. Victim generate a signature to approve contract C in chain A.
  2. Malicious actor generate identifical contract C in chain B.
  3. The HolographERC20.sol token is deployed cross chain to chain B.
  4. Malicious actor copy the signature that victim used to approval contract C in Chain A to approve contract C in chain B.
  5. Malicious actor steal user's fund in chain B.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend add chain Id in the signature schedma to avoid the multichain replay.

function getChainID() internal iew returns (uint256) {
    uint256 id;
    assembly {
        id := chainid()
    }
    return id;
}
    bytes32 structHash = keccak256(
      abi.encode(
        0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9,
		    getChainID(),
        account,
        spender,
        amount,
        _useNonce(account),
        deadline
      )
    );
    bytes32 hash = _hashTypedDataV4(structHash);
    address signer = ECDSA.recover(hash, v, r, s);
    require(signer == account, "ERC20: invalid signature");
    if (_isEventRegistered(HolographERC20Event.beforeApprove)) {
      require(SourceERC20().beforeApprove(account, spender, amount));
    }
@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Oct 24, 2022
code423n4 added a commit that referenced this issue Oct 24, 2022
@gzeoneth
Copy link
Member

Duplicate of #178

@gzeoneth gzeoneth marked this as a duplicate of #178 Oct 31, 2022
@gzeoneth gzeoneth added the duplicate This issue or pull request already exists label Oct 31, 2022
@gzeoneth gzeoneth added the invalid This doesn't seem right label Nov 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate This issue or pull request already exists invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants