Fix: Block blacklisted spender in blacklist issuance tokens

src/external/token/ERC20IssuanceUpgradeable_Blacklist_v1.sol

@@ -229,6 +229,26 @@ contract ERC20IssuanceUpgradeable_Blacklist_v1 is
         super._update(from_, to_, amount_);
     }
 
+    /// @notice Internal hook to also block a blacklisted spender from moving
+    ///         tokens via an allowance.
+    /// @dev    The `_update` hook only sees `from` and `to`, so without this a
+    ///         blacklisted address holding an allowance could still spend it
+    ///         through `transferFrom` (or any allowance-based spend). Checking
+    ///         the spender here closes that path.
+    /// @param  owner_ Address the allowance is drawn from.
+    /// @param  spender_ Address spending the allowance.
+    /// @param  value_ Amount being spent.
+    function _spendAllowance(address owner_, address spender_, uint value_)
+        internal
+        virtual
+        override
+    {
+        if (isBlacklisted(spender_)) {
+            revert ERC20Issuance_Blacklist_BlacklistedAddress(spender_);
+        }
+        super._spendAllowance(owner_, spender_, value_);
+    }
+
     /// @notice Internal function to set a blacklist manager.
     /// @param  manager_ Address to set as blacklist manager.
     /// @param  allowed_ Whether to grant or revoke the blacklist manager role.

src/external/token/ERC20Issuance_Blacklist_v1.sol

@@ -222,6 +222,26 @@ contract ERC20Issuance_Blacklist_v1 is
         super._update(from_, to_, amount_);
     }
 
+    /// @notice Internal hook to also block a blacklisted spender from moving
+    ///         tokens via an allowance.
+    /// @dev    The `_update` hook only sees `from` and `to`, so without this a
+    ///         blacklisted address holding an allowance could still spend it
+    ///         through `transferFrom` (or any allowance-based spend). Checking
+    ///         the spender here closes that path.
+    /// @param  owner_ Address the allowance is drawn from.
+    /// @param  spender_ Address spending the allowance.
+    /// @param  value_ Amount being spent.
+    function _spendAllowance(address owner_, address spender_, uint value_)
+        internal
+        virtual
+        override
+    {
+        if (isBlacklisted(spender_)) {
+            revert ERC20Issuance_Blacklist_BlacklistedAddress(spender_);
+        }
+        super._spendAllowance(owner_, spender_, value_);
+    }
+
     /// @notice Internal function to set a blacklist manager.
     /// @param  manager_ Address to set as blacklist manager.
     /// @param  allowed_ Whether to grant or revoke the blacklist manager role.

test/unit/external/token/ERC20IssuanceUpgradeable_blacklist_v1.t.sol

@@ -954,6 +954,148 @@ contract ERC20IssuanceUpgradeable_Blacklist_v1_Test is Test {
         );
     }
 
+    /*  Test: blacklisted spender via transferFrom
+        ├── Given a holder approved a spender and the spender is blacklisted
+        │   └── When transferFrom is called by the spender
+        │       └── Then it should revert with BlacklistedAddress(spender)
+        └── Given the spender is not blacklisted
+            └── When transferFrom is called by the spender
+                └── Then it should move the holder's tokens
+    */
+
+    function testTransferFrom_revertGivenSpenderIsBlacklisted(uint amount_)
+        public
+    {
+        amount_ = bound(amount_, 1, uint(type(uint).max - 1));
+        address holder = makeAddr("holder");
+        address spender = makeAddr("spender");
+        address dest = makeAddr("dest");
+
+        _fundAddress(holder, amount_);
+        vm.prank(holder);
+        token.approve(spender, amount_);
+
+        // Blacklist the spender only. Holder and dest stay clean, so the
+        // `_update(from, to)` checks would pass; the spender check must catch it.
+        _blacklistAddress(spender);
+
+        vm.prank(spender);
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                IERC20Issuance_Blacklist_v1
+                    .ERC20Issuance_Blacklist_BlacklistedAddress
+                    .selector,
+                spender
+            )
+        );
+        token.transferFrom(holder, dest, amount_);
+
+        // Funds must not have moved.
+        assertEq(token.balanceOf(holder), amount_, "holder balance untouched");
+        assertEq(token.balanceOf(dest), 0, "dest received nothing");
+    }
+
+    function testTransferFrom_worksGivenSpenderIsNotBlacklisted(uint amount_)
+        public
+    {
+        amount_ = bound(amount_, 1, uint(type(uint).max - 1));
+        address holder = makeAddr("holder");
+        address spender = makeAddr("spender");
+        address dest = makeAddr("dest");
+
+        _fundAddress(holder, amount_);
+        vm.prank(holder);
+        token.approve(spender, amount_);
+
+        vm.prank(spender);
+        token.transferFrom(holder, dest, amount_);
+
+        assertEq(token.balanceOf(holder), 0, "holder fully spent");
+        assertEq(token.balanceOf(dest), amount_, "dest received funds");
+    }
+
+    // The blacklist check runs before OZ's infinite-allowance short-circuit,
+    // so an unlimited approval to a blacklisted spender is still blocked.
+    function testTransferFrom_revertGivenBlacklistedSpenderWithInfiniteAllowance(
+        uint amount_
+    ) public {
+        amount_ = bound(amount_, 1, uint(type(uint).max - 1));
+        address holder = makeAddr("holder");
+        address spender = makeAddr("spender");
+        address dest = makeAddr("dest");
+
+        _fundAddress(holder, amount_);
+        vm.prank(holder);
+        token.approve(spender, type(uint).max);
+
+        _blacklistAddress(spender);
+
+        vm.prank(spender);
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                IERC20Issuance_Blacklist_v1
+                    .ERC20Issuance_Blacklist_BlacklistedAddress
+                    .selector,
+                spender
+            )
+        );
+        token.transferFrom(holder, dest, amount_);
+
+        assertEq(token.balanceOf(holder), amount_, "holder balance untouched");
+        assertEq(token.balanceOf(dest), 0, "dest received nothing");
+    }
+
+    /*  Test: minter spendAllowance wrapper also goes through the spender check
+        ├── Given the spender is blacklisted
+        │   └── When the minter calls spendAllowance()
+        │       └── Then it should revert with BlacklistedAddress(spender)
+        └── Given the spender is not blacklisted
+            └── When the minter calls spendAllowance()
+                └── Then it should consume the allowance
+    */
+
+    function testSpendAllowanceWrapper_revertGivenSpenderIsBlacklisted(
+        uint amount_
+    ) public {
+        amount_ = bound(amount_, 1, uint(type(uint).max - 1));
+        address holder = makeAddr("holder");
+        address spender = makeAddr("spender");
+
+        vm.prank(holder);
+        token.approve(spender, amount_);
+        _blacklistAddress(spender);
+
+        // Caller is the test contract, which is the minter (set in setUp).
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                IERC20Issuance_Blacklist_v1
+                    .ERC20Issuance_Blacklist_BlacklistedAddress
+                    .selector,
+                spender
+            )
+        );
+        token.spendAllowance(holder, spender, amount_);
+
+        assertEq(
+            token.allowance(holder, spender), amount_, "allowance untouched"
+        );
+    }
+
+    function testSpendAllowanceWrapper_worksGivenSpenderIsNotBlacklisted(
+        uint amount_
+    ) public {
+        amount_ = bound(amount_, 1, uint(type(uint).max - 1));
+        address holder = makeAddr("holder");
+        address spender = makeAddr("spender");
+
+        vm.prank(holder);
+        token.approve(spender, amount_);
+
+        token.spendAllowance(holder, spender, amount_);
+
+        assertEq(token.allowance(holder, spender), 0, "allowance consumed");
+    }
+
     // ================================================================================
     // Helper Functions
 

test/unit/external/token/ERC20Issuance_blacklist_v1.t.sol

@@ -917,6 +917,148 @@ contract ERC20Issuance_Blacklist_v1_Test is Test {
         );
     }
 
+    /*  Test: blacklisted spender via transferFrom
+        ├── Given a holder approved a spender and the spender is blacklisted
+        │   └── When transferFrom is called by the spender
+        │       └── Then it should revert with BlacklistedAddress(spender)
+        └── Given the spender is not blacklisted
+            └── When transferFrom is called by the spender
+                └── Then it should move the holder's tokens
+    */
+
+    function testTransferFrom_revertGivenSpenderIsBlacklisted(uint amount_)
+        public
+    {
+        amount_ = bound(amount_, 1, uint(type(uint).max - 1));
+        address holder = makeAddr("holder");
+        address spender = makeAddr("spender");
+        address dest = makeAddr("dest");
+
+        _fundAddress(holder, amount_);
+        vm.prank(holder);
+        token.approve(spender, amount_);
+
+        // Blacklist the spender only. Holder and dest stay clean, so the
+        // `_update(from, to)` checks would pass; the spender check must catch it.
+        _blacklistAddress(spender);
+
+        vm.prank(spender);
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                IERC20Issuance_Blacklist_v1
+                    .ERC20Issuance_Blacklist_BlacklistedAddress
+                    .selector,
+                spender
+            )
+        );
+        token.transferFrom(holder, dest, amount_);
+
+        // Funds must not have moved.
+        assertEq(token.balanceOf(holder), amount_, "holder balance untouched");
+        assertEq(token.balanceOf(dest), 0, "dest received nothing");
+    }
+
+    function testTransferFrom_worksGivenSpenderIsNotBlacklisted(uint amount_)
+        public
+    {
+        amount_ = bound(amount_, 1, uint(type(uint).max - 1));
+        address holder = makeAddr("holder");
+        address spender = makeAddr("spender");
+        address dest = makeAddr("dest");
+
+        _fundAddress(holder, amount_);
+        vm.prank(holder);
+        token.approve(spender, amount_);
+
+        vm.prank(spender);
+        token.transferFrom(holder, dest, amount_);
+
+        assertEq(token.balanceOf(holder), 0, "holder fully spent");
+        assertEq(token.balanceOf(dest), amount_, "dest received funds");
+    }
+
+    // The blacklist check runs before OZ's infinite-allowance short-circuit,
+    // so an unlimited approval to a blacklisted spender is still blocked.
+    function testTransferFrom_revertGivenBlacklistedSpenderWithInfiniteAllowance(
+        uint amount_
+    ) public {
+        amount_ = bound(amount_, 1, uint(type(uint).max - 1));
+        address holder = makeAddr("holder");
+        address spender = makeAddr("spender");
+        address dest = makeAddr("dest");
+
+        _fundAddress(holder, amount_);
+        vm.prank(holder);
+        token.approve(spender, type(uint).max);
+
+        _blacklistAddress(spender);
+
+        vm.prank(spender);
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                IERC20Issuance_Blacklist_v1
+                    .ERC20Issuance_Blacklist_BlacklistedAddress
+                    .selector,
+                spender
+            )
+        );
+        token.transferFrom(holder, dest, amount_);
+
+        assertEq(token.balanceOf(holder), amount_, "holder balance untouched");
+        assertEq(token.balanceOf(dest), 0, "dest received nothing");
+    }
+
+    /*  Test: minter spendAllowance wrapper also goes through the spender check
+        ├── Given the spender is blacklisted
+        │   └── When the minter calls spendAllowance()
+        │       └── Then it should revert with BlacklistedAddress(spender)
+        └── Given the spender is not blacklisted
+            └── When the minter calls spendAllowance()
+                └── Then it should consume the allowance
+    */
+
+    function testSpendAllowanceWrapper_revertGivenSpenderIsBlacklisted(
+        uint amount_
+    ) public {
+        amount_ = bound(amount_, 1, uint(type(uint).max - 1));
+        address holder = makeAddr("holder");
+        address spender = makeAddr("spender");
+
+        vm.prank(holder);
+        token.approve(spender, amount_);
+        _blacklistAddress(spender);
+
+        // Caller is the test contract, which is the minter (set in setUp).
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                IERC20Issuance_Blacklist_v1
+                    .ERC20Issuance_Blacklist_BlacklistedAddress
+                    .selector,
+                spender
+            )
+        );
+        token.spendAllowance(holder, spender, amount_);
+
+        assertEq(
+            token.allowance(holder, spender), amount_, "allowance untouched"
+        );
+    }
+
+    function testSpendAllowanceWrapper_worksGivenSpenderIsNotBlacklisted(
+        uint amount_
+    ) public {
+        amount_ = bound(amount_, 1, uint(type(uint).max - 1));
+        address holder = makeAddr("holder");
+        address spender = makeAddr("spender");
+
+        vm.prank(holder);
+        token.approve(spender, amount_);
+
+        token.spendAllowance(holder, spender, amount_);
+
+        assertEq(token.allowance(holder, spender), 0, "allowance consumed");
+    }
+
     // ================================================================================
     // Helper Functions