code-423n4 / 2023-01-opensea-findings

0 stars 0 forks source link

Falsification of conduit keys #71

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/conduit/ConduitController.sol#L56-L84

Vulnerability details

Impact

In function createConduit, https://github.com/ProjectOpenSea/seaport/blob/5de7302bc773d9821ba4759e47fc981680911ea0/contracts/conduit/ConduitController.sol#L56 users can falsify their keys by providing their address in the first 20bytes of the conduitkey. Being able to create as many conduits as someone wants.

Proof of Concept

Given the check that opensea is making:

 if (address(uint160(bytes20(conduitKey))) != msg.sender) { 

you can derive that if you try different random keys with your address in front, all will pass:

pragma solidity ^0.8.0;

contract seaport2{

function testKey(bytes32 conduitKey)public pure returns(address user){
   user = address(uint160(bytes20(conduitKey)));
}
}

//address: 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
//key1: 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4000000000000000000000000
//key2: 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4000000000000006969696969
//user in both cases: 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4                     

Tools Used

manual

Recommended Mitigation Steps

Add a maximum amount of conduits that a user can create or make a whitelisting process in order to create a conduit.

0age commented 1 year ago

contested; every account has 2^96 conduit keys namespaced to them and is free to deploy as many conduits as they like (though it'd be a huge waste for them to do so). whitelisting would introduce unnecessary central control over the protocol.

HickupHH3 commented 1 year ago

Feature, not bug...?

c4-judge commented 1 year ago

HickupHH3 marked the issue as unsatisfactory: Invalid