Zellic - Audit Reports

Category: Business Logic

Escrow counterparty misassignment

Medium Impact

Medium Severity

Medium Likelihood

Description

The signDealAndPay and signAndFinalizeDeal functions in DealManager pass msg.sender to updateEscrow, causing the escrow to store the transaction caller as the counterparty instead of the authenticated signer.

function signDealAndPay(
    address signer,
    bytes32 agreementId,
    bytes memory signature,
    string[] memory partyValues,
    bool _fillUnallocated,
    string memory name,
    string memory secret
) public {
    // [...]
    updateEscrow(agreementId, msg.sender, name);
    // [...]
}

function signAndFinalizeDeal(
    address signer,
    bytes32 agreementId,
    string[] memory partyValues,
    bytes memory signature,
    bool _fillUnallocated,
    string memory name,
    string memory secret
) public {
    // [...]
    updateEscrow(agreementId, msg.sender, name);
    // [...]
}

function updateEscrow(bytes32 agreementId, address counterParty, string memory buyerName) internal {
    Escrow storage escrow = LexScrowStorage.getEscrow(agreementId);
    escrow.counterParty = counterParty;
    [...]
}

When the deal finalizes, the corp assets transfer to the caller instead of the authenticated signer.

function finalizeEscrow(bytes32 agreementId) internal {
    // [...]
    for(uint256 i = 0; i < escrow.corpAssets.length; i++) {
        if(escrow.corpAssets[i].tokenType == TokenType.ERC20) {
            IERC20(escrow.corpAssets[i].tokenAddress).safeTransfer(escrow.counterParty, escrow.corpAssets[i].amount);
        }
        else if(escrow.corpAssets[i].tokenType == TokenType.ERC721) {
            IERC721(escrow.corpAssets[i].tokenAddress).safeTransferFrom(address(this), escrow.counterParty, escrow.corpAssets[i].tokenId);
        }
        else if(escrow.corpAssets[i].tokenType == TokenType.ERC1155) {
            IERC1155(escrow.corpAssets[i].tokenAddress).safeTransferFrom(address(this), escrow.counterParty, escrow.corpAssets[i].tokenId, escrow.corpAssets[i].amount, "");
        }
    }
}

Impact

An attacker who observes a legitimate investor's signDealAndPay or signAndFinalizeDeal transaction in the mempool can front-run the transaction and submit the same call with the victim's signature. Although the attacker must provide payment, they can receive the corp assets without signing the agreement.

Recommendations

We recommend setting the escrow counterparty to the validated signer.

-updateEscrow(agreementId, msg.sender, name);
+updateEscrow(agreementId, signer, name);

Remediation

This issue has been acknowledged by MetaLex, and a fix was implemented in commit 4c808dc8↗.