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

Bridge::withdrawTokens bypass MAX_PAYLOAD_LENGTH limit

Dowsers

Summary

The Starklane bridge contract contains a vulnerability that allows an attacker to bypass the MAX_PAYLOAD_LENGTH = 300 check in the Bridge:withdrawTokens function. This could potentially lead to the processing of oversized payloads, causing unexpected behavior or denial of service.

Vulnerability Details

The withdrawTokens function in the Starklane contract (Bridge.sol) is designed to process token withdrawals from L2 to L1. It uses a constant MAX_PAYLOAD_LENGTH (set to 300) to limit the size of incoming payloads using Bridge::depositTokens. However, this check is implemented in the Bridge::depositTokens function but not in the Bridge:withdrawTokens function.

Assuming you have followed the contest setup, copy and past the following test in the Bridge.t.sol file, and run the following command : forge test --mt testWithdraw10000Tokens -vv

function testWithdraw10000Tokens() public {
// amount of tokens to withdraw
uint256 numTokens = 10000;
​
// Request creation
felt252 header = Protocol.requestHeaderV1(CollectionType.ERC721, false, false);
uint256[] memory tokenIds = new uint256[](numTokens);
for(uint256 i = 0; i < numTokens; i++) {
tokenIds[i] = i + 1;
}
​
Request memory req = Request ({
header: header,
hash: 0x1,
collectionL1: address(0x0),
collectionL2: Cairo.snaddressWrap(0x123),
ownerL1: bob,
ownerL2: Cairo.snaddressWrap(0x789),
name: "Ark Project",
symbol: "ArkP",
uri: "htpps://arkproject.com",
tokenIds: tokenIds,
tokenValues: new uint256[](0),
tokenURIs: new string[](0),
newOwners: new uint256[](0)
});
uint256[] memory reqSerialized = Protocol.requestSerialize(req);
bytes32 msgHash = computeMessageHashFromL2(reqSerialized);
uint256 payload = reqSerialized.length;
​
//Simulate the message coming from L2
uint256[] memory hashes = new uint256[](1);
hashes[0] = uint256(msgHash);
IStarknetMessagingLocal(snCore).addMessageHashesFromL2(hashes);
​
// withdraw tokens
address collection = IStarklane(bridge).withdrawTokens(reqSerialized);
​
// Check if the tokens were transferred to the correct owner
for(uint256 i = 0; i < numTokens; i++) {
assertEq(IERC721(collection).ownerOf(tokenIds[i]), bob);
}
// Check if the collection is withelist
assert(IStarklane(bridge).isWhiteListed(collection));
​
// Asserting the payload request is below limit
console.log("Payload", payload);
console.log("Max payload", MAX_PAYLOAD_LENGTH);
assertGe(payload, MAX_PAYLOAD_LENGTH);
}

You should get this input :

Ran 1 test for test/Bridge.t.sol:BridgeTest
[PASS] testWithdraw10000Tokens() (gas: 357851945)
Logs:
Payload 20020
Max payload 300
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 557.14ms (495.79ms CPU time)

The purpose of testWithdraw10000Tokens function is to see if we can withdraw Tokens beyond MAX_PAYLOAD_LENGTH = 300. As you can see above the request's payload is 20020 way above the limit (300) and we can indeed remove all the tokens from the request.

Impact

The ability to bypass the MAX_PAYLOAD_LENGTH limit in the Bridge::withdrawTokens function exposes the Starklane bridge to severe Denial of Service (DoS) attacks, primarily through the exploitation of block gas limits. This vulnerability presents a critical threat to the operational stability and throughput of the bridge.

The test demonstrates that withdrawing 10,000 tokens consumes 357,851,945 gas. Ethereum's current block gas limit is around 30,000,000 gas. A single withdrawal transaction of this size would consume approximately 11.93 entire blocks' worth of gas.

An attacker could submit multiple such transactions, potentially consuming dozens of blocks. Legitimate transactions would face extreme delays or consistent failures due to lack of block space. Given that a normal Ethereum transaction uses about 21,000 gas, one of these malicious withdrawals is equivalent to about 17,040 standard transactions.

Users might be forced to pay exorbitant fees to have any chance of their transactions being processed. This could make the bridge economically non-viable for most users during the attack. Attackers could time these massive withdrawals to coincide with known high-traffic periods.

Tools Used

Foundry

Recommendations

Implement a payload size check in the Bridge::depositTokens function:

function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
if (!_enabled) {
revert BridgeNotEnabledError();
}
​
// Header is always the first uint256 of the serialized request.
uint256 header = request[0];
​
// Any error or permission fail in the message consumption will cause a revert.
// After message being consumed, it is considered legit and tokens can be withdrawn.
if (Protocol.canUseWithdrawAuto(header)) {
// 2024-03-19: disabled autoWithdraw after audit report
// _consumeMessageAutoWithdraw(_starklaneL2Address, request);
revert NotSupportedYetError();
} else {
_consumeMessageStarknet(_starknetCoreAddress, _starklaneL2Address, request);
}
Request memory req = Protocol.requestDeserialize(request, 0);
​
++ if (req.length >= MAX_PAYLOAD_LENGTH) {
++ revert TooManyTokensError();
++ }
// ... (rest of the function)
}
Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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