Zellic - Audit Reports

Category: Business Logic

Malicious registry signing blocks deal participation

Medium Impact

Medium Severity

Medium Likelihood

Description

The DealManager contract expects deals to be signed through signDealAndPay or signAndFinalizeDeal, which calls signContractFor in CyberAgreementRegistry to sign the agreement:

function signDealAndPay(
    address signer, bytes32 agreementId,
    bytes memory signature, string[] memory partyValues,
    bool _fillUnallocated, string memory name,
    string memory secret
) public {
    // [...]
    ICyberAgreementRegistry(LexScrowStorage.getDealRegistry())
        .signContractFor(signer, agreementId, partyValues, 
            signature, _fillUnallocated, secret);
    // [...]
}

function signAndFinalizeDeal(
    address signer, bytes32 agreementId, string[] memory partyValues,
    bytes memory signature, bool _fillUnallocated,
    string memory name, string memory secret
) public {
    // [...]
    if(!ICyberAgreementRegistry(LexScrowStorage.getDealRegistry())
        .hasSigned(agreementId, signer))
        ICyberAgreementRegistry(LexScrowStorage.getDealRegistry())
            .signContractFor(signer, agreementId, partyValues, 
                signature, _fillUnallocated, secret);
    // [...]
}

However, any party can call signContractFor in CyberAgreementRegistry directly. For an open deal, one signature slot in the registry remains available for a signer. When the first signer claims the slot, other investors cannot sign the contract and participate in the deal.

function signContractFor(
    address signer,
    bytes32 contractId,
    string[] memory partyValues,
    bytes calldata signature,
    bool fillUnallocated, // to fill a 0 address or not
    string memory secret
) public {
	// [...]
    if (!isParty(contractId, signer)) {
        if (
            agreementData.secretHash > 0 &&
            keccak256(abi.encode(secret)) != agreementData.secretHash
        ) revert InvalidSecret();
        // Not a named party, so check if there's an open slot
        uint256 firstOpenPartyIndex = getFirstOpenPartyIndex(contractId);
        if (firstOpenPartyIndex == 0 || !fillUnallocated)
            revert NotAParty();
        // There is a spare slot, assign the sender to this slot.
        agreementData.parties[firstOpenPartyIndex] = signer;
        agreementsForParty[agreementData.parties[firstOpenPartyIndex]].push(
                contractId
            );
    }
    // [...]
    if (totalSignatures == agreementData.parties.length) {
        if (agreementData.finalizer == address(0)) {
            agreementData.finalized = true;
            emit ContractFinalized(contractId, msg.sender, timestamp);
        }

        emit ContractFullySigned(contractId, timestamp);
    }
}

A malicious counterparty can sign the deal directly through the registry, consuming the limited signature slots without performing the escrow payment. Because the slot is filled, legitimate investors cannot participate in the deal.

Impact

For open deals, an attacker can prevent investors from participating in deals by consuming the signature slot in the registry.

Recommendations

Restrict signContractFor so that only DealManager or designated contracts can invoke it.

Remediation

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

MetaLex provided the following response to this finding:

We will check that sender matches the signature sent in so that someone can't steal a signature and front run, but we don't want "open deal" agreements to be restricted to finalizers as they are used for other use cases.
If a deal gets griefed, we would suggest to send a closed offer to the party attempting to invest.