code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/ConduitController.sol#L57-L104 https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/ConduitController.sol#L118-L179

Vulnerability details

Impact

Anyone can call the createConduit() function in the ConduitController contract to create new channels and set the conduit owner. This is dangerous because a hacker can create a new conduit and set himself as the owner of the conduit. The hacker can use the updateChannel() function to set the legit Seaport contract as a channel, and divert user traffic to use the conduit contract that he owns. This could be done by either promoting it directly on social media / Discord / Telegram etc., or via phishing attack, where users thought they were interacting with the legit Seaport conduit contract but are in fact interacting with the hacker owned conduit contract.

This is like a time bomb ready to explode any time, as the hacker can freely add channels to the conduit any time, and a malicious channel could drain all users' funds that have approved the hacker's conduit contract for token balance.

This could also enable potential double spending attack, if multiple Seaport contracts are added as channel for the conduit contract, the same signed order could be used on both Seaport contracts. Please refer to the separate finding titled "More than 1 Channel Set in the Conduit Might Result in More Tokens Transferred From the Offerer Than Intended" for details.

Proof of Concept

The createConduit() function does not have any access restriction modifier, as shown below. A hacker could therefore freely create channels and set himself as the conduit owner.

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/ConduitController.sol#L57-L104

Then the hacker could use the updateChannel() function to set Seaport as a channel for his conduit, including setting multiple Seaport contracts as valid channels. The hacker has now planted seed for rug pull. As more users approve his conduit for assets, the hacker can choose to add a malicious channel (either EOA or a contract) any time and drain the fund that users have approved his conduit contract for.

https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/conduit/ConduitController.sol#L118-L179

Tools Used

Manual Review

Recommended Mitigation Steps

The createConduit() function is a privileged function and should have an access restriction modifier. It would be very confusing to users with numerous conduit contracts that use Seaport as a channel. I'd recommend adding access restriction to the createConduit() function, either via multisig + timelock, or DAO + timelock.

0age commented 2 years ago

There is no double-spend possibility as each deployed Seaport instance includes the contract address and chainId in its domain separator. Furthermore, this was explicitly marked as out-of-scope; conduits, and more generally granting token approvals, are inherently dangerous (which is important for users to be aware of)

HardlyDifficult commented 2 years ago

As a protocol, these contracts aim to be unopinionated and therefore createConduit should not be privileged. It is important that users can trust the owner of the conduit they choose to use.

0xleastwood commented 2 years ago

Again, the README lists this as out-of-scope:

As a malicious or vulnerable conduit owner may set a channel on a conduit that allows for approved tokens to be taken at will, we make the assumption in the context of this contest that only the Seaport contract will be added as a channel to any conduit.

code-423n4 / 2022-05-opensea-seaport-findings