Reentrancy

[code423n4]

Lines of code

https://github.com/ProjectOpenSea/seaport/blob/f402dac8b3faabdb8420d31d46759f47c9d74b7d/contracts/conduit/ConduitController.sol#L256-L281

Vulnerability details

Impact

An attacker can be the owner of conduitcontroller contract

Proof of Concept

function acceptOwnership has re-entrancy vulnerability Look at this : https://github.com/ProjectOpenSea/seaport/blob/f402dac8b3faabdb8420d31d46759f47c9d74b7d/contracts/conduit/ConduitController.sol#L256-L281

function acceptOwnership Executes twice to make the caller as owner. Look at this

=>>   emit OwnershipTransferred( 
             conduit, 
=>>       _conduits[conduit].owner, 
              msg.sender 
         ); 

         // Set the caller as the owner of the conduit. 
=>>       _conduits[conduit].owner = msg.sender; 
     }

This is what makes re-entrancy vulnerability, Attackers will take advantage of this vulnerability to take over ownership.

The acceptOwnership function should only end in

 emit OwnershipTransferred( 
             conduit, 
             _conduits[conduit].owner, 
             msg.sender 
         ); 

Tools Used

Manual review

Recommended Mitigation Steps

Remove this :  _conduits[conduit].owner = msg.sender; And create the guardians, and then require the guardians to accept the ownership

[0age]

contested; these aren't external calls, they're accessing local state

[HickupHH3]

No ability to re-enter since it doesn't have external calls.

[c4-judge]

HickupHH3 marked the issue as unsatisfactory: Insufficient quality