Skip to content

Commit

Permalink
modified the constructor
Browse files Browse the repository at this point in the history
  • Loading branch information
PoulavBhowmick03 committed Oct 2, 2024
1 parent 09ae423 commit f37f3ae
Show file tree
Hide file tree
Showing 3 changed files with 2,165 additions and 17 deletions.
3 changes: 3 additions & 0 deletions apps/blockchain/ethereum/src/Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ contract Starklane is IStarklaneEvent, UUPSOwnableProxied, StarklaneState, Stark
bool _enabled;
bool _whiteListEnabled;

constructor() {
_disableInitializers();
}

/**
@notice Initializes the implementation, only callable once.
Expand Down
71 changes: 54 additions & 17 deletions apps/blockchain/ethereum/test/Bridge.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,13 @@ contract BridgeTest is Test, IStarklaneEvent {
address erc721C1;
address erc1155C1;
address snCore;

address collectionL1= address(0x1);
snaddress collectionL2 = Cairo.snaddressWrap(0x2);

address impl;

address payable internal alice;
address payable internal bob;

address attacker = address(0xBEEF);

//
function setUp() public {
Users genusers = new Users();
Expand All @@ -48,7 +47,7 @@ contract BridgeTest is Test, IStarklaneEvent {

snCore = address(new StarknetMessagingLocal());

address impl = address(new Starklane());
impl = address(new Starklane());

bytes memory dataInit = abi.encodeWithSelector(
Starklane.initialize.selector,
Expand All @@ -64,9 +63,47 @@ contract BridgeTest is Test, IStarklaneEvent {
IStarklane(bridge).enableBridge(true);
}

function testAttackerCanInitializeImplementation() public {
// This test demonstrates the vulnerability before the fix
vm.prank(attacker);

bytes memory data = abi.encode(
attacker, // owner
address(0x0), // starknetCoreAddress
uint256(0x0), // starklaneL2Address
uint256(0x0) // starklaneL2Selector
);

// Attacker calls initialize on the implementation contract
Starklane(impl).initialize(data);

// Verify that the attacker is now the owner of the implementation contract
address owner = Starklane(impl).owner();
assertEq(owner, attacker, "Attacker should be the owner");
}

function testAttackerCannotInitializeImplementationAfterFix() public {
// After the fix, the attacker should not be able to initialize the implementation contract
// Re-deploy the fixed implementation contract
impl = address(new Starklane());

vm.prank(attacker);

bytes memory data = abi.encode(
attacker, // owner
address(0x0), // starknetCoreAddress
uint256(0x0), // starklaneL2Address
uint256(0x0) // starklaneL2Selector
);

// Expect the call to revert with "Initializable: contract is already initialized"
vm.expectRevert("Initializable: contract is already initialized");
Starklane(impl).initialize(data);
}

//
function testFail_invalidIds() public {
uint256[] memory ids = new uint256[](0);
uint256;

uint256 salt = 0x1;
snaddress to = Cairo.snaddressWrap(0x1);
Expand All @@ -82,7 +119,7 @@ contract BridgeTest is Test, IStarklaneEvent {

//
function testFail_invalidSalt() public {
uint256[] memory ids = new uint256[](2);
uint256;
ids[0] = 0;
ids[1] = 9;

Expand All @@ -100,7 +137,7 @@ contract BridgeTest is Test, IStarklaneEvent {

//
function testFail_invalidL2Owner() public {
uint256[] memory ids = new uint256[](0);
uint256;

uint256 salt = 0x0;
snaddress to = Cairo.snaddressWrap(0x0);
Expand All @@ -115,7 +152,7 @@ contract BridgeTest is Test, IStarklaneEvent {
}

function test_L2OwnerOverflow() public {
uint256[] memory ids = new uint256[](0);
uint256;

uint256 salt = 0x0;
snaddress to = snaddress.wrap(SN_MODULUS);
Expand All @@ -134,7 +171,7 @@ contract BridgeTest is Test, IStarklaneEvent {
function test_depositTokenERC721() public {
IERC721MintRangeFree(erc721C1).mintRangeFree(alice, 0, 10);

uint256[] memory ids = new uint256[](2);
uint256;
ids[0] = 0;
ids[1] = 9;

Expand All @@ -160,7 +197,7 @@ contract BridgeTest is Test, IStarklaneEvent {

IERC721MintRangeFree(erc721C1).mintRangeFree(alice, 0, 10);

uint256[] memory ids = new uint256[](2);
uint256;
ids[0] = 0;
ids[1] = 9;

Expand All @@ -183,7 +220,7 @@ contract BridgeTest is Test, IStarklaneEvent {
function test_depositTokenERC721_notWhiteListed() public {
IERC721MintRangeFree(erc721C1).mintRangeFree(alice, 0, 10);

uint256[] memory ids = new uint256[](2);
uint256;
ids[0] = 0;
ids[1] = 9;

Expand All @@ -209,7 +246,7 @@ contract BridgeTest is Test, IStarklaneEvent {
function test_depositTokenERC721_whiteListed() public {
IERC721MintRangeFree(erc721C1).mintRangeFree(alice, 0, 10);

uint256[] memory ids = new uint256[](2);
uint256;
ids[0] = 0;
ids[1] = 9;

Expand All @@ -235,19 +272,19 @@ contract BridgeTest is Test, IStarklaneEvent {

// Test event related to whitelist
function test_events_whiteList() public {
vm.expectEmit(bridge);
vm.expectEmit(true, true, true, true, bridge);
emit WhiteListUpdated(false);
IStarklane(bridge).enableWhiteList(false);

vm.expectEmit(bridge);
vm.expectEmit(true, true, true, true, bridge);
emit WhiteListUpdated(true);
IStarklane(bridge).enableWhiteList(true);

vm.expectEmit(bridge);
vm.expectEmit(true, true, true, true, bridge);
emit CollectionWhiteListUpdated(erc721C1, true);
IStarklane(bridge).whiteList(erc721C1, true);

vm.expectEmit(bridge);
vm.expectEmit(true, true, true, true, bridge);
emit CollectionWhiteListUpdated(erc721C1, false);
IStarklane(bridge).whiteList(erc721C1, false);

Expand Down
Loading

0 comments on commit f37f3ae

Please sign in to comment.