From 09ae423bc6707da21a86273733b5b2d3528b1b20 Mon Sep 17 00:00:00 2001
From: Sandeep chauhan <92181599+ryzen-xp@users.noreply.github.com>
Date: Mon, 30 Sep 2024 15:33:09 +0530
Subject: [PATCH] =?UTF-8?q?Fix=20:=20#234=20Missing=20checks=20for=20addre?=
 =?UTF-8?q?ss(0x0)=20when=20updating=20address=20stat=E2=80=A6=20(#240)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Fix : #234 Missing checks for address(0x0) when updating address state variables

Summary of Changes Made
Input Validation in setL1L2CollectionMapping:
Added checks to ensure collectionL1 is not address(0) and collectionL2 is not 0 to prevent invalid mappings.
Event Emission:
Emitted L1L2CollectionMappingUpdated after setting the mapping for transparency.

* add setL1L2collectionMapping test in  Bridge.t.sol

* Update Bridge.sol

* Bridge.sol

* Update Bridge.t.sol

rename test functions

* Update Bridge.sol

remove require and use if  and revert or error

* Update Bridge.t.sol

* Update Bridge.t.sol

vm.expectRevert(InvalidL2AddressError.selector);
---
 apps/blockchain/ethereum/src/Bridge.sol    | 10 +++++++-
 apps/blockchain/ethereum/test/Bridge.t.sol | 30 ++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/apps/blockchain/ethereum/src/Bridge.sol b/apps/blockchain/ethereum/src/Bridge.sol
index e62c7ce9..663a4232 100644
--- a/apps/blockchain/ethereum/src/Bridge.sol
+++ b/apps/blockchain/ethereum/src/Bridge.sol
@@ -21,6 +21,8 @@ error CollectionMappingError();
 error NotWhiteListedError();
 error BridgeNotEnabledError();
 error TooManyTokensError();
+error InvalidL1AddressError();
+error InvalidL2AddressError();
 
 uint256 constant MAX_PAYLOAD_LENGTH = 300;
 
@@ -261,7 +263,7 @@ contract Starklane is IStarklaneEvent, UUPSOwnableProxied, StarklaneState, Stark
         address collectionL1 = req.collectionL1;
         for (uint256 i = 0; i < req.tokenIds.length; i++) {
             uint256 id = req.tokenIds[i];
-            _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
+             _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
         }
     }
 
@@ -370,6 +372,12 @@ contract Starklane is IStarklaneEvent, UUPSOwnableProxied, StarklaneState, Stark
         snaddress collectionL2,
         bool force
     ) external onlyOwner {
+          if (collectionL1 == address(0x0)) {
+              revert InvalidL1AddressError();
+            }
+          if (snaddress.unwrap(collectionL2) == 0x0) {
+               revert InvalidL2AddressError();
+            }
         _setL1L2AddressMapping(collectionL1, collectionL2, force);
         emit L1L2CollectionMappingUpdated(collectionL1, snaddress.unwrap(collectionL2));
     }
diff --git a/apps/blockchain/ethereum/test/Bridge.t.sol b/apps/blockchain/ethereum/test/Bridge.t.sol
index 7ca9df29..14605f9f 100644
--- a/apps/blockchain/ethereum/test/Bridge.t.sol
+++ b/apps/blockchain/ethereum/test/Bridge.t.sol
@@ -29,6 +29,10 @@ contract BridgeTest is Test, IStarklaneEvent {
     address erc1155C1;
     address snCore;
 
+    address collectionL1= address(0x1);
+    snaddress collectionL2 = Cairo.snaddressWrap(0x2);
+
+
     address payable internal alice;
     address payable internal bob;
 
@@ -600,5 +604,31 @@ contract BridgeTest is Test, IStarklaneEvent {
         assert(payload.length == reqContent.length);
         return (nonce, payload);
     }
+     
+    // here is test   for set L1 L2 collection 
+   
+    function test_SetL1L2CollectionMapping() public {
+    
+        // Check for the event emission
+        vm.expectEmit(true, true, true, true);
+        emit L1L2CollectionMappingUpdated(collectionL1, snaddress.unwrap(collectionL2));
+       IStarklane(bridge).setL1L2CollectionMapping(collectionL1, collectionL2, false);
+    }
+
+    function test_SetL1L2CollectionMappingWith_InvalidL1Address() public {
+         vm.expectRevert(InvalidL1AddressError.selector);
+       IStarklane(bridge).setL1L2CollectionMapping(address(0), collectionL2, false);
+    }
+
+    function test_SetL1L2CollectionMappingWith_InvalidL2Address() public {
+        vm.expectRevert(InvalidL2AddressError.selector);
+        IStarklane(bridge).setL1L2CollectionMapping(collectionL1, Cairo.snaddressWrap(0), false);
+    }
+
+    function test_SetL1L2CollectionMappingWith_InvalidUnwrappedL2Address() public {
+        snaddress invalidCollectionL2 = Cairo.snaddressWrap(0x0); 
+        vm.expectRevert(InvalidL2AddressError.selector);
+        IStarklane(bridge).setL1L2CollectionMapping(collectionL1, invalidCollectionL2, false);
+    }
 
 }