ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Potential injection of malicious code through the `withdrawTokens` function

Bellatrix

Useful Github Link

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Bridge.sol#L183-L192

Summary

The withdrawTokens function in Bridge.sol allows for the deployment of arbitrary code if collectionL1 does not exist. This could potentially enable malicious actors to introduce harmful code through the contract.

Vulnerability Details

When withdrawTokens is called with an L1 address that doesn't exist, it triggers the deployment of a new contract using parameters passed to the function. There are no safeguards in place to validate that these parameters do not contain malicious bytecode or scripts. This allows an attacker to inject arbitrary code into the newly deployed contract, potentially compromising the entire bridge system.

if (collectionL1 == address(0x0)) {
if (ctype == CollectionType.ERC721) {
collectionL1 = _deployERC721Bridgeable(
req.name,
req.symbol,
req.collectionL2,
req.hash
);
// update whitelist if needed
_whiteListCollection(collectionL1, true);

Proof of Concept

A test case was been added to Bridge.t.sol. This test showcases how an attacker can embed javascript code that could potentially affect either the contract code or the web interface displaying the code.

The string "><script>alert('XSS')</script>" is a classic example of a Cross-Site Scripting (XSS) attack. XSS allows attackers to inject malicious scripts into web pages viewed by other users.

function test_withdrawTokensERC721MaliciousContractDeploy() public {
felt252 header = Protocol.requestHeaderV1(CollectionType.ERC721, false, false);
Request memory req = buildMaliciousRequestDeploy(header, 888, bob);
uint256[] memory reqSerialized = Protocol.requestSerialize(req);
bytes32 msgHash = computeMessageHashFromL2(reqSerialized);
​
// Simulate the message coming from Starknet
uint256[] memory hashes = new uint256[](1);
hashes[0] = uint256(msgHash);
IStarknetMessagingLocal(snCore).addMessageHashesFromL2(hashes);
​
address collection = IStarklane(bridge).withdrawTokens(reqSerialized);
assertEq(IERC721Metadata(collection).symbol(), "MAL", "Unexpected contract symbol");
assertEq(IERC721Metadata(collection).name(), "><script>alert('XSS')</script>","Unexpected contract symbol");
​
}
function buildMaliciousRequestDeploy(felt252 header, uint256 tokenId, address to, bytes memory maliciousCode) internal pure returns (Request memory) {
Request memory req;
req.header = header;
req.hash = 0x1234; // Example hash
req.collectionL1 = address(0); // Indicates new deployment
req.collectionL2 = snaddress.wrap(0x5678); // Example L2 address
req.ownerL1 = to;
req.ownerL2 = snaddress.wrap(uint256(uint160(to)));
req.name = "><script>alert('XSS')</script>";
req.symbol = "MAL";
req.uri = "";
req.tokenIds = new uint256[](1);
req.tokenIds[0] = tokenId;
req.tokenValues = new uint256[](0);
req.tokenURIs = new string[](0);
req.newOwners = new uint256[](0);
​
// Include malicious bytecode in the request
req.name = string(maliciousCode);
​
return req;
}

Impact

The severity is critical. This vulnerability can lead to significant financial losses and compromise the integrity of the entire bridge system. Attackers could:

  1. Injection of malware into user wallets or interfaces

  2. Manipulation of bridge logic to favor attackers

Tools Used

  • Manual code review

  • Test suites

Recommendations

  • Thoroughly validate all input parameters, especially those used in contract deployment.

  • Enforce strict limits on string lengths and content for fields like name and symbol.

Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.