fix: audit FIND-015

.changeset/fix-batch-transfer-compliance-checks.md

@@ -0,0 +1,9 @@
+---
+"@hashgraph/asset-tokenization-contracts": patch
+---
+
+Fix: `batchTransfer` now validates sender and each destination against the control list and compliance module independently.
+
+Previously, `batchTransfer` used `onlyCompliant(sender, address(0), false)` for the sender and called `checkCompliance(address(0), to, false)` per destination, passing `address(0)` as the counterpart and `0` as the value in both cases. The sender was never validated as an individual account (only its compliance-contract approval was checked), and each per-destination compliance call omitted the actual sender and amount.
+
+The sender is now checked via `onlyAccountCompliant(sender)`, which validates recovery status and control-list membership before any token movement. Inside the loop, each destination is checked with `checkAccountCompliance(to)` (recovery + control list) followed by `checkTransferCompliance(sender, to, amounts[i])`, which calls the compliance module with the real sender address and transfer amount.

packages/ats/contracts/contracts/domain/asset/ERC1594StorageWrapper.sol

@@ -42,6 +42,7 @@ struct ERC1594Storage {
 
 /**
  * @title ERC1594StorageWrapper
+ * @author Asset Tokenization Studio Team
  * @notice Library providing the core issuance, redemption, and compliance
  * checking logic for the ERC1594 token standard. Handles storage management,
  * balance mutations via ERC20/ERC1410 wrappers, and multi-layered access
@@ -156,20 +157,6 @@ library ERC1594StorageWrapper {
         checkIdentity(from, to);
     }
 
-    /**
-     * @notice Reverts if the transfer between `from` and `to` fails
-     * compliance checks. Optionally checks sender compliance.
-     * @dev Forwards to `checkCompliance`. Used when the sender is not
-     * necessarily the `from` parameter.
-     * @param from Source address.
-     * @param to Destination address.
-     * @param checkSender Whether to perform compliance validation on the
-     * current `msg.sender`.
-     */
-    function requireCompliant(address from, address to, bool checkSender) internal view {
-        checkCompliance(from, to, checkSender);
-    }
-
     /**
      * @notice Reverts if either `from` or `to` is listed as a recovered
      * wallet.
@@ -313,16 +300,57 @@ library ERC1594StorageWrapper {
      * @param checkSender Whether to validate compliance of `msg.sender`.
      */
     function checkCompliance(address from, address to, bool checkSender) internal view {
+        checkCompliance(from, to, 0, checkSender);
+    }
+
+    /**
+     * @notice Reverts if the transfer between `from` and `to` fails
+     * compliance checks. Optionally checks sender compliance.
+     * @dev Delegates to `_isCompliant` and reverts with the encoded reason
+     * code and details on failure.
+     * @param from Source address (may be zero).
+     * @param to Destination address (may be zero).
+     * @param value Amount tried to be transferred.
+     * @param checkSender Whether to validate compliance of `msg.sender`.
+     */
+    function checkCompliance(address from, address to, uint256 value, bool checkSender) internal view {
         (bool isCompliant_, , bytes32 reasonCode, bytes memory details) = _isCompliant(
             from,
             to,
-            0,
+            value,
             EvmAccessors.getMsgSender(),
             checkSender
         );
         if (!isCompliant_) LowLevelCall.revertWithData(bytes4(reasonCode), details);
     }
 
+    /**
+     * @notice Reverts if `account` fails individual account compliance checks.
+     * @dev Delegates to `_isAccountCompliant`, which validates the account against
+     * the recovery registry and the control list. Zero address is always allowed.
+     * @param account Address to validate.
+     */
+    function checkAccountCompliance(address account) internal view {
+        (bool isCompliant_, , bytes32 reasonCode, bytes memory details) = _isAccountCompliant(account);
+        if (!isCompliant_) LowLevelCall.revertWithData(bytes4(reasonCode), details);
+    }
+
+    /**
+     * @notice Reverts if the compliance module disallows a transfer from `from`
+     * to `to` for `value` tokens.
+     * @dev Delegates to `_validateTransferCompliance`, which calls the compliance
+     * contract's `canTransfer(from, to, value)` via staticcall. Use this when only
+     * the compliance module check is required, bypassing recovery and control list
+     * validation.
+     * @param from Source address.
+     * @param to Destination address.
+     * @param value Amount of tokens to transfer.
+     */
+    function checkTransferCompliance(address from, address to, uint256 value) internal view {
+        (bool isCompliant_, , bytes32 reasonCode, bytes memory details) = _validateTransferCompliance(from, to, value);
+        if (!isCompliant_) LowLevelCall.revertWithData(bytes4(reasonCode), details);
+    }
+
     /**
      * @notice Returns the ERC1594 storage slot using the predefined
      * position constant.
@@ -376,12 +404,57 @@ library ERC1594StorageWrapper {
             );
     }
 
+    /**
+     * @notice Validates compliance for the sender involved in a transfer
+     * or redemption.
+     * @dev Checks the sender against recovery, control list, and compliance contract.
+     * @param from Source address (may be zero).
+     * @param to Destination address (may be zero).
+     * @param value Transfer amount.
+     * @param sender Address of the operator initiating the operation.
+     * @return status True if all compliance checks pass.
+     * @return statusCode EIP1066 status byte.
+     * @return reasonCode Selector of the blocking error.
+     * @return details Encoded error data (empty on success).
+     */
+    function _isSenderCompliant(
+        address from,
+        address to,
+        uint256 value,
+        address sender
+    ) private view returns (bool status, bytes1 statusCode, bytes32 reasonCode, bytes memory details) {
+        (status, statusCode, reasonCode, details) = _validateAccountForTransfer(sender, abi.encode(sender));
+        if (!status) return (status, statusCode, reasonCode, details);
+        (status, statusCode, reasonCode, details) = _validateSenderCompliance(sender, from, to, value);
+        if (!status) return (status, statusCode, reasonCode, details);
+    }
+
+    /**
+     * @notice Validates compliance for the 'from' involved in a transfer
+     * or redemption.
+     * @dev Checks the from against recovery, control list, and compliance contract.
+     * @param account Account address (may be zero).
+     * @return status True if all compliance checks pass.
+     * @return statusCode EIP1066 status byte.
+     * @return reasonCode Selector of the blocking error.
+     * @return details Encoded error data (empty on success).
+     */
+    function _isAccountCompliant(
+        address account
+    ) private view returns (bool status, bytes1 statusCode, bytes32 reasonCode, bytes memory details) {
+        if (account != address(0)) {
+            (status, statusCode, reasonCode, details) = _validateAccountForTransfer(account, abi.encode(account));
+            if (!status) return (status, statusCode, reasonCode, details);
+        }
+        return (true, Eip1066.SUCCESS, bytes32(0), EMPTY_BYTES);
+    }
+
     /**
      * @notice Validates compliance for all parties involved in a transfer
      * or redemption.
      * @dev Checks the sender (if flagged), `from`, and `to` against
      * recovery, control list, and compliance contract. The compliance
-     * contract is called via staticcall with `canTransfer`.
+     * contract is called via static call with `canTransfer`.
      * @param from Source address (may be zero).
      * @param to Destination address (may be zero).
      * @param value Transfer amount.
@@ -400,19 +473,13 @@ library ERC1594StorageWrapper {
         bool checkSender
     ) private view returns (bool status, bytes1 statusCode, bytes32 reasonCode, bytes memory details) {
         if (checkSender) {
-            (status, statusCode, reasonCode, details) = _validateAccountForTransfer(sender, abi.encode(sender));
-            if (!status) return (status, statusCode, reasonCode, details);
-            (status, statusCode, reasonCode, details) = _validateSenderCompliance(sender, from, to, value);
-            if (!status) return (status, statusCode, reasonCode, details);
-        }
-        if (from != address(0)) {
-            (status, statusCode, reasonCode, details) = _validateAccountForTransfer(from, abi.encode(from));
-            if (!status) return (status, statusCode, reasonCode, details);
-        }
-        if (to != address(0)) {
-            (status, statusCode, reasonCode, details) = _validateAccountForTransfer(to, abi.encode(to));
+            (status, statusCode, reasonCode, details) = _isSenderCompliant(from, to, value, sender);
             if (!status) return (status, statusCode, reasonCode, details);
         }
+        (status, statusCode, reasonCode, details) = _isAccountCompliant(from);
+        if (!status) return (status, statusCode, reasonCode, details);
+        (status, statusCode, reasonCode, details) = _isAccountCompliant(to);
+        if (!status) return (status, statusCode, reasonCode, details);
         (status, statusCode, reasonCode, details) = _validateTransferCompliance(from, to, value);
         if (!status) return (status, statusCode, reasonCode, details);
         return (true, Eip1066.SUCCESS, bytes32(0), EMPTY_BYTES);

packages/ats/contracts/contracts/domain/orchestrator/ClearingOps.sol

@@ -425,7 +425,7 @@ library ClearingOps {
 
         // Verify identity and compliance for transfers to different addresses
         TokenCoreOps.checkIdentity(_id.tokenHolder, transferData.destination);
-        TokenCoreOps.checkCompliance(_id.tokenHolder, transferData.destination, false);
+        TokenCoreOps.checkCompliance(_id.tokenHolder, transferData.destination, transferData.amount, false);
 
         // Notify compliance module (same pattern as HoldStorageWrapper and ERC1410StorageWrapper)
         if (_id.partition == _DEFAULT_PARTITION && ERC3643StorageWrapper.erc3643Storage().compliance != address(0)) {
@@ -466,7 +466,7 @@ library ClearingOps {
 
         // Approve: _verify identity/compliance (tokens are burned, no transfer back)
         TokenCoreOps.checkIdentity(_id.tokenHolder, address(0));
-        TokenCoreOps.checkCompliance(_id.tokenHolder, address(0), false);
+        TokenCoreOps.checkCompliance(_id.tokenHolder, address(0), 0, false);
     }
 
     /**

packages/ats/contracts/contracts/domain/orchestrator/TokenCoreOps.sol

@@ -152,8 +152,8 @@ library TokenCoreOps {
         ERC1594StorageWrapper.checkIdentity(_from, _to);
     }
 
-    function checkCompliance(address _from, address _to, bool _checkSender) public view {
-        ERC1594StorageWrapper.checkCompliance(_from, _to, _checkSender);
+    function checkCompliance(address _from, address _to, uint256 amount, bool _checkSender) public view {
+        ERC1594StorageWrapper.checkCompliance(_from, _to, amount, _checkSender);
     }
 
     // Internal functions (inlined into calling StorageWrappers)

packages/ats/contracts/contracts/facets/batchTransfer/BatchTransfer.sol

@@ -30,12 +30,13 @@ abstract contract BatchTransfer is IBatchTransfer, Modifiers {
         onlyUnProtectedPartitionsOrWildCardRole
         onlyClearingDisabled
         onlyIdentifiedAddresses(EvmAccessors.getMsgSender(), address(0))
-        onlyCompliant(EvmAccessors.getMsgSender(), address(0), false)
+        onlyAccountCompliant(EvmAccessors.getMsgSender())
     {
         uint256 length = _toList.length;
         for (uint256 i; i < length; ) {
             ERC1594StorageWrapper.checkIdentity(address(0), _toList[i]);
-            ERC1594StorageWrapper.checkCompliance(address(0), _toList[i], false);
+            ERC1594StorageWrapper.checkAccountCompliance(_toList[i]);
+            ERC1594StorageWrapper.checkTransferCompliance(EvmAccessors.getMsgSender(), _toList[i], _amounts[i]);
             unchecked {
                 ++i;
             }

packages/ats/contracts/contracts/services/asset/ComplianceModifiers.sol

@@ -64,7 +64,18 @@ abstract contract ComplianceModifiers {
      * @param checkSender Whether to also validate msg.sender against compliance rules.
      */
     modifier onlyCompliant(address from, address to, bool checkSender) {
-        ERC1594StorageWrapper.checkCompliance(from, to, checkSender);
+        ERC1594StorageWrapper.checkCompliance(from, to, 0, checkSender);
+        _;
+    }
+
+    /**
+     * @dev Modifier that verifies whether account satisfies
+     * all compliance rules enforced by the compliance module.
+     *
+     * @param account The account address to validate.
+     */
+    modifier onlyAccountCompliant(address account) {
+        ERC1594StorageWrapper.checkAccountCompliance(account);
         _;
     }
 }

packages/ats/contracts/test/contracts/integration/batchTransfer/batchTransfer.test.ts

@@ -80,6 +80,10 @@ describe("BatchTransfer Tests", () => {
         role: ATS_ROLES.PROTECTED_PARTITIONS_ROLE,
         members: [signer_A.address],
       },
+      {
+        role: ATS_ROLES.CONTROL_LIST_ROLE,
+        members: [signer_A.address],
+      },
     ]);
 
     await asset.grantRole(ATS_ROLES.ISSUER_ROLE, signer_A.address);
@@ -217,6 +221,58 @@ describe("BatchTransfer Tests", () => {
           "ComplianceNotAllowed",
         );
       });
+
+      describe("ControlList", () => {
+        it("GIVEN a blacklisted sender WHEN batchTransfer THEN transaction fails with AccountIsBlocked", async () => {
+          await asset.addToControlList(signer_E.address);
+
+          const toList = [signer_F.address];
+          const amounts = [transferAmount];
+
+          await expect(asset.connect(signer_E).batchTransfer(toList, amounts)).to.be.revertedWithCustomError(
+            asset,
+            "AccountIsBlocked",
+          );
+        });
+
+        it("GIVEN a blacklisted destination WHEN batchTransfer THEN transaction fails with AccountIsBlocked", async () => {
+          await asset.addToControlList(signer_F.address);
+
+          const toList = [signer_D.address, signer_F.address];
+          const amounts = [transferAmount, transferAmount];
+
+          await expect(asset.connect(signer_E).batchTransfer(toList, amounts)).to.be.revertedWithCustomError(
+            asset,
+            "AccountIsBlocked",
+          );
+        });
+      });
+
+      describe("Recovery", () => {
+        it("GIVEN a recovered sender WHEN batchTransfer THEN transaction fails with WalletRecovered", async () => {
+          await asset.recoveryAddress(signer_E.address, signer_D.address, ethers.ZeroAddress);
+
+          const toList = [signer_F.address];
+          const amounts = [transferAmount];
+
+          await expect(asset.connect(signer_E).batchTransfer(toList, amounts)).to.be.revertedWithCustomError(
+            asset,
+            "WalletRecovered",
+          );
+        });
+
+        it("GIVEN a recovered destination WHEN batchTransfer THEN transaction fails with WalletRecovered", async () => {
+          await asset.recoveryAddress(signer_F.address, signer_D.address, ethers.ZeroAddress);
+
+          const toList = [signer_F.address];
+          const amounts = [transferAmount];
+
+          await expect(asset.connect(signer_E).batchTransfer(toList, amounts)).to.be.revertedWithCustomError(
+            asset,
+            "WalletRecovered",
+          );
+        });
+      });
     });
   });