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

HolographFactory.sol cross-chain deployment is vulnerable to front-running: User control address a in chain A does not mean user control address a in chain B if the address is not a EOA account #171

Closed
code423n4 opened this issue Oct 24, 2022 · 2 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working edited-by-warden invalid This doesn't seem right resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) responded The Holograph team has reviewed and responded sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/HolographERC721.sol#L132
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/HolographERC20.sol#L138
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/PA1D.sol#L120
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/HolographERC721.sol#L242

Vulnerability details

Impact

HolographFactory.sol cross-chain deploy is vulnerable to front-running: User control address A in one chain does not mean user control address in chain B.

The can result in compromised admin addrss and malicious actor can take control over the contract in another blockchain.

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

HolographFactory.sol is used to deploy identical smart contract in any EVM blockchain. I want to quote

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.

However, the identical smart contract implies that the configuration is the same across chain. I intend to prove this approach is not safe.

because User control address A in one chain does not mean user control address in chain B.

the user may control smart contract in chain A, but has no control over the same address in chain B.

We can refer to wintermute hack back in June

https://rekt.news/wintermute-rekt

the result is Wintermute have lost 20M OP token.

The funds were supposed to be sent to Wintermute by the Optimism Foundation in an agreement to act as a market maker ahead of the OP token launch.

But Wintermute provided the address of their multisig on Ethereum as the destination address on Optimism - an address they did not control.

Once the tokens had been sent, they were sitting out in the open, ready to be taken by anyone who spotted them…

The hint was the fact that the address corresponded to a Gnosis Safe proxy on mainnet, but had no contract deployed to the Optimism address.

Nobody could take control of the address as an EOA, that would require the private key.

However, there was a way to access the funds; anyone could take control of the address by deploying a Gnosis Safe proxy to it.

then hacker front-run the wintermute and deploy the gnosis safe proxy contract to control the 20M OP before the wintermute rescue the fund.

Wintermute takes control of the multsig wallet in ethreum, they did not take control over the same control in optimism, which is the root cause of the hacker.

This example prove that User control address A in one chain does not mean user control address in chain B.

let us walk through a concrete case in POC in Holograph context!

Proof of Concept

The users want to deploy the HolographERC721.sol cross-chain from chain ethereum to chain Polygon.

/**
 * @title Holograph Bridgeable ERC-721 Collection
 * @author CXIP-Labs
 * @notice A smart contract for minting and managing Holograph Bridgeable ERC721 NFTs.
 * @dev The entire logic and functionality of the smart contract is self-contained.
 */
contract HolographERC721 is Admin, Owner, HolographERC721Interface, Initializable {

the contract inheirs from admin

abstract contract Admin {
  /**
   * @dev bytes32(uint256(keccak256('eip1967.Holograph.admin')) - 1)
   */
  bytes32 constant _adminSlot = 0x3f106594dc74eeef980dae234cde8324dc2497b13d27a0c59e55bd2ca10a07c9;

then admin slot is set in the HolographERC721.sol contructor

https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/enforcer/HolographERC721.sol#L242

assembly {
  sstore(_ownerSlot, caller())
  sourceContract := sload(_sourceContractSlot)
}

well, let us say in blockchain ethereum, the admin address is 0xe7dd363c482272b9395A183f7D48FA1De98778Df, it is multisg wallet address.

  1. the victim wants to deploy a cross-chain NFT in polygon, he calls the function to HolographFactory.sol.
  2. the admin slot in polygon is also set to 0xe7dd363c482272b9395A183f7D48FA1De98778Df.
  3. the victim later realize that he does not own the multisig wallet address in polygon, he wants to deploy the genosis proxy to take control of the admin contract.
  4. The hacker steps in, front-run the victim, deploy the genosis proxy contract before the victim did and take admin control of the NFT in polygon.

same issue applies in

/**
 * @title PA1D (CXIP)
 * @author CXIP-Labs
 * @notice A smart contract for providing royalty info, collecting royalties, and distributing it to configured payout wallets.
 * @dev This smart contract is not intended to be used directly. Apply it to any of your ERC721 or ERC1155 smart contracts through a delegatecall fallback.
 */
contract PA1D is Admin, Owner, Initializable {

the PA1D royality management use _payoutAddressesSlot

/**
* @dev bytes32(uint256(keccak256('eip1967.Holograph.PA1D.payout.addresses')) - 1)
*/
bytes32 constant _payoutAddressesSlot = 0x700a541bc37f227b0d36d34e7b77cc0108bde768297c6f80f448f380387371df;

the payout address could be a EOA account, could a smart contract address. If it is a smart contract address,

the user can control the smart contract address in chain A to receive the payment, but has no control over the addrses in blockchain B.

Tools Used

Manual review

Recommended Mitigation Steps

We recommend the protocol make sure the cross-chain deployed contract's admin slot,

owner slot, _payoutAddressesSlot and other crucial slot that has access control or receive payments

only allow EOA account.

And emphasis this point in document heavily, very very heavily.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Oct 24, 2022
code423n4 added a commit that referenced this issue Oct 24, 2022
@code423n4 code423n4 changed the title HolographFactory.sol cross-chain deployment is vulnerable to front-running: User control address A in one chain does not mean user control address in chain B HolographFactory.sol cross-chain deployment is vulnerable to front-running: User control address a in chain A does not mean user control address a in chain B if the address is not a EOA account Oct 24, 2022
@gzeoneth gzeoneth added the question Further information is requested label Oct 31, 2022
@gzeoneth
Copy link
Member

The deployment require a valid signature
https://github.com/code-423n4/2022-10-holograph/blob/f8c2eae866280a1acfdc8a8352401ed031be1373/contracts/HolographFactory.sol#L194
which is only possible if it is an EOA.

@gzeoneth gzeoneth added sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue and removed question Further information is requested labels Oct 31, 2022
@alexanderattar
Copy link

Signature prevents configurations changes.

@alexanderattar alexanderattar added the responded The Holograph team has reviewed and responded label Nov 8, 2022
@alexanderattar alexanderattar added the resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) label Nov 15, 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
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working edited-by-warden invalid This doesn't seem right resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) responded The Holograph team has reviewed and responded sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

3 participants