CodeHawks Escrow Contract - Competition Details
Cyfrin
Foundry
40,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Escrow - Lack of set Arbiter function

victorygod

Summary

The addition of the arbiter mechanism in the escrow contract provides an extra layer of security and trust in the transaction process. It allows for the appointment of a trusted third party to resolve disputes between the buyer and seller.

Vulnerability Details

The "set arbiter" function allows the buyer and seller to choose an arbiter based on their specific requirements and preferences. It gives them the ability to select someone they trust or someone with expertise in the relevant industry or domain.

Impact

Without the ability to set a new arbiter, the parties are bound to the initially appointed arbiter throughout the entire duration of the contract. This lack of flexibility can be problematic if the arbiter becomes unavailable, biased, or unresponsive. It limits the parties' ability to address any concerns or issues that may arise during the transaction process.

Tools Used

Manual Analysis

Recommendations

something like this could be implemented

/// @notice Sets a new arbiter `_arbiter`. His address should be approved by both parties.
function setArbiter(
bytes32 _tradeRecordId,
address _seller,
address _buyer,
address _arbiter,
uint _expireAtBlock,
uint _salt,
bytes _bothSignatures
)
external
onlyRelay
returns (uint)
{
// Calculate the hash of the escrow
bytes32 _escrowHash = _getEscrowHash(_tradeRecordId, _seller, _buyer);
​
// Calculate the encoded salt with the escrow hash
bytes32 _encodedSaltWithEscrow = _getEncodedSaltWithEscrowHash(_salt, _escrowHash);
​
// Check if the escrow exists
require(_getEscrowExists(_escrowHash), "DISPUTED_ESCROW_SHOULD_EXIST");
​
// Check if the salt with escrow mapping does not already exist
require(!_saltWithEscrow2flagMapping[_encodedSaltWithEscrow], "DISPUTED_SUCH_SALT_SHOULD_NOT_EXIST");
​
// Create the message to be signed
bytes memory _message = abi.encodePacked(
_escrowHash,
address(this),
msg.sig,
_arbiter,
_expireAtBlock,
_salt
);
​
// Get the signers from the signatures
address[] memory _signers = Signatures.getSignersFromSignatures(keccak256(_message), _bothSignatures);
​
// Check if the signers are the buyer and seller
require(
_signers.length == 2 &&
(
(_signers[0] == _seller && _signers[1] == _buyer) ||
(_signers[0] == _buyer && _signers[1] == _seller)
),
"DISPUTED_SIGNERS_SHOULD_BE_BUYER_AND_SELLER"
);
​
// Check if the transaction is not expired
require(block.number < _expireAtBlock, "DISPUTED_TX_SHOULD_NOT_BE_EXPIRED");
​
// Set the salt with escrow mapping to true
_saltWithEscrow2flagMapping[_encodedSaltWithEscrow] = true;
​
// Set the arbiter for the escrow
_setEscrowArbiter(_escrowHash, _arbiter);
​
// Emit an event to indicate the transfer of arbiter
_emitArbiterTransferred(_escrowHash, _arbiter);
​
return OK;
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.