CallWhitelistApprovals Manual Review Findings

CWA-01M: Insecure Whitelist Potential

Type	Severity	Location
Logical Fault		CallWhitelistApprovals.sol:L26, L51-L54

Description:

The CallWhitelistApprovals contract implementation is presently insecure when utilized with an AssetVault in the LoanCore context as it permits spenders to be set before an AssetVault is escrowed, essentially compromising the funds held within the AssetVault after the loan has been created.

Impact:

The current implementation of AssetVault in conjunction with CallWhitelistApprovals is incompatible with the Arcade XYZ lending system.

Example:

Copy
42/**
43 * @notice Sets approval status of a given token for a spender. Note that this is
44 *         NOT a token approval - it is permission to create a token approval from
45 *         the asset vault.
46 *
47 * @param token                The token approval to set.
48 * @param spender              The token spender.
49 * @param _isApproved          Whether the spender should be approved.
50 */
51function setApproval(address token, address spender, bool _isApproved) external onlyOwner {
52    approvals[token][spender] = _isApproved;
53    emit ApprovalSet(msg.sender, token, spender, _isApproved);
54}

Recommendation:

We advise a new mapping to be introduced that marks a particular asset / collateral as "dirty". In essence, whenever CallWhitelistApprovals::setApproval is called it will mark the specified token as "dirty" permitting on-chain code to validate whether the token may be compromised within an AssetVault.

A new ISignatureVerifier can thus be introduced to the codebase that evaluates predicates guaranteeing that the assets are not "dirty" and thus safe to assume that they will remain within the escrowed AssetVault.

Alleviation (7a4e1dc948):

The Arcade XYZ team specified that they wish to retain the current functionality of the whitelist in place and that it is up to integrators as well as counterparties to validate the suitability of an AssetVault as collateral, potentially by utilizing specialized verifier contracts.

We would like to note that the inclusion of a programmatic check is meant to prevent on-chain race conditions whereby a user manually assesses an AssetVault as viable and its status its changed via an on-chain race.

In any case, we consider dedicated verifiers to be a suitable resolution to the vulnerability described and we would like to advise the Arcade XYZ to consider including them as part of the "standard" Arcade XYZ library.

As the Arcade XYZ team stated that they do not wish to pursue any additional immediate action, we consider this exhibit acknowledged albeit to be re-evaluated by the Arcade XYZ team.

Alleviation (45ccaa43fa):

The Arcade XYZ team re-evaluated this exhibit and has opted to retain their acknowledgement as programmatic checks would hinder the flexibility of the protocol and predicates have been introduced to it for this very purpose (i.e. additional validations of a particular loan agreement).

As such, we consider this exhibit acknowledged with no further action pending from the Arcade XYZ team.