GatedMintEscrow Manual Review Findings

GME-01M: Inexistent Strategy Integration

Type	Severity	Location
Standard Conformity		GatedMintEscrow.sol:L146, L177, L231, L256

Description:

The GatedMintEscrow contract is meant to be integrated by a strategy, however, no strategy presently integrates it.

Specifically, all deposit acceptance and refund flows rely on a strategy invoking it yet no contract in scope invokes those functions. There are several hidden assumptions and potential vulnerabilities that might arise from how strategies integrate with the GatedMintEscrow contract which cannot be surfaced without a reference implementation.

Example:

Copy
229function refundDeposit(bytes32 depositId) external {
230    // Only strategy can call this function
231    if (msg.sender != strategy) revert Unauthorized();

Recommendation:

We advise a reference implementation of a strategy to be introduced to the audit scope so as to properly validate the GatedMintEscrow contract's proper operation.

As an example, a smart contract relying on batch acceptance or refund flows might be susceptible to an irrecoverable Denial-of-Service attack if a user reclaims their deposit directly on the GatedMintEscrow contract.

Alleviation (e4d6885477438291b0d3ca477fab7e088522967b):

The Sova Network team evaluated this exhibit and clarified that the strategy is meant to be interacted with via manually crafted payloads through the BasicStrategy::call function.

As such interactions are impossible to validate reliably on-chain and involve off-chain processes, we consider all integrations to be validated atomically and strongly recommend the off-chain processes to be closely evaluated by the Sova Network team for any potential issues.

GME-02M: Insecure Type Casting

Type	Severity	Location
Mathematical Operations		GatedMintEscrow.sol:L128

Description:

The expirationTime variable is downcast from a uint256 data type to the uint96 data type without any bound checking, permitting casting overflows to occur.

Impact:

Although unlikely, a value that exceeds type(uint96).max as an expirationTime will result in an underflow without yielding an error which we consider incorrect.

Example:

Copy
113function handleDepositReceived(
114    bytes32 depositId,
115    address depositor,
116    address recipient,
117    uint256 amount,
118    uint256 expirationTime
119) external {
120    // Only GatedMintRWA token can call this function
121    if (msg.sender != token) revert Unauthorized();
122
123    // Store the deposit data
124    pendingDeposits[depositId] = PendingDeposit({
125        depositor: depositor,
126        recipient: recipient,
127        assetAmount: amount,
128        expirationTime: uint96(expirationTime),
129        state: DepositState.PENDING,
130        atRound: currentRound
131    });
132
133    // Update accounting
134    totalPendingAssets += amount;
135    userPendingAssets[depositor] += amount;
136
137    emit DepositReceived(depositId, depositor, recipient, amount, expirationTime);
138}

Recommendation:

We advise the code to ensure that the expirationTime fits within the uint96 data type, preventing potential manipulations of the recorded expiration time of a deposit.

Alleviation (e4d6885477438291b0d3ca477fab7e088522967b):

The Sova Network team evaluated this exhibit and opted to acknowledge it as an aberrant value would solely arise from a misconfiguration of the GatedMintRWA contract, rendering this exhibit acknowledged.

GME-03M: Inexistent Slippage Protection

Type	Severity	Location
Logical Fault		GatedMintEscrow.sol:L113-L119, L144-L169, L175-L223

Description:

The GatedMintEscrow contract meant to be interfaced by the GatedMintRWA contract is supposed to be part of a ReportedStrategy, effectively one whose evaluation might change through oracle measurements (or other mechanisms).

The present delayed deposit mechanism does not allow the depositors to specify an upper price they are willing to accept (or other control mechanism), effectively rendering their deposits susceptible to arbitrary slippage.

Impact:

All delayed deposits are subject to arbitrary fluctuations in the artificial exchange rate of the EIP-4626 vault which relies on a BaseReporter, such as a PriceOracleReporter.

Example:

Copy
105/**
106 * @notice Receive a deposit from the GatedMintRWA token
107 * @param depositId Unique identifier for the deposit
108 * @param depositor Address that initiated the deposit
109 * @param recipient Address that will receive shares if approved
110 * @param amount Amount of assets deposited
111 * @param expirationTime Time after which deposit can be reclaimed
112 */
113function handleDepositReceived(
114    bytes32 depositId,
115    address depositor,
116    address recipient,
117    uint256 amount,
118    uint256 expirationTime
119) external {
120    // Only GatedMintRWA token can call this function
121    if (msg.sender != token) revert Unauthorized();
122
123    // Store the deposit data
124    pendingDeposits[depositId] = PendingDeposit({
125        depositor: depositor,
126        recipient: recipient,
127        assetAmount: amount,
128        expirationTime: uint96(expirationTime),
129        state: DepositState.PENDING,
130        atRound: currentRound
131    });
132
133    // Update accounting
134    totalPendingAssets += amount;
135    userPendingAssets[depositor] += amount;
136
137    emit DepositReceived(depositId, depositor, recipient, amount, expirationTime);
138}
139
140/**
141 * @notice Accept a pending deposit
142 * @param depositId The deposit ID to accept
143 */
144function acceptDeposit(bytes32 depositId) external {
145    // Only strategy can call this function
146    if (msg.sender != strategy) revert Unauthorized();
147
148    PendingDeposit storage deposit = pendingDeposits[depositId];
149    if (deposit.depositor == address(0)) revert DepositNotFound();
150    if (deposit.state != DepositState.PENDING) revert DepositNotPending();
151
152    // Mark as accepted
153    deposit.state = DepositState.ACCEPTED;
154
155    // Update accounting
156    totalPendingAssets -= deposit.assetAmount;
157    userPendingAssets[deposit.depositor] -= deposit.assetAmount;
158
159    // Increment the round number, even for a single deposit
160    currentRound++;
161
162    // Transfer assets to the strategy
163    SafeTransferLib.safeTransfer(asset, strategy, deposit.assetAmount);
164
165    // Tell the GatedMintRWA token to mint shares
166    GatedMintRWA(token).mintShares(deposit.recipient, deposit.assetAmount);
167
168    emit DepositAccepted(depositId, deposit.recipient, deposit.assetAmount);
169}

Recommendation:

We advise the contracts to be refactored so as to permit an upper price bound to be specified that the depositors are willing to accept, effectively converting the GatedMintEscrow contract into an oracle-specific or at least price-specific contract module.

Alleviation (e4d6885477438291b0d3ca477fab7e088522967b):

The Sova Network team evaluated this exhibit and consider the vulnerability to be correct. As such, they have opted to introduce a FIXME comment on the GatedMintRWA contract regarding this issue and to acknowledge the exhibit at this stage.