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

Users can sidestep Gas fee payment because it is not properly enforced for L1<>L2 deposit transactions

audinarey

Summary

Users can make deposit transacions with less than the required amount of gas.

Vulnerability Details

When users call Bridge::depositTokens(...) function on L1 to initiate transfers to starktnet, they are required to send a message fee (gas fee) for the execution of the transaction.
The problem is that because a minimum fee requirement is not enforced, when a user deposits on L1 they can call Bridge::depositTokens(...)with 1 wei and the transaction will succeed becuase as shown on L131 below it only checks if msg.value > 0. So using msg.value = 1 will succeed.

File: Bridge.sol
078: function depositTokens( // @audit deposit with malformed header and 1 wei gass fee to DOS the bridge
079: uint256 salt,
080: address collectionL1,
081: snaddress ownerL2,
082: uint256[] calldata ids,
083: bool useAutoBurn
084: ) // @audit about 230502 gas is used for this transavtion as shown in the test_depositTokenERC721Nogas() test case however
085: external
086: payable
087: {
​
091: ....
137: @> IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}( // @audit 2) excess gas fee is not refunded to sender of an L1->L2 message
138: snaddress.unwrap(_starklaneL2Address),
139: felt252.unwrap(_starklaneL2Selector),
140: payload
141: );
142:
143: emit DepositRequestInitiated(req.hash, block.timestamp, payload);
144: }
​
​
​
File: StarknetMessaging.sol
126: function sendMessageToL2(
127: uint256 toAddress,
128: uint256 selector,
129: uint256[] calldata payload
130: ) external payable override returns (bytes32, uint256) {
131: @> require(msg.value > 0, "L1_MSG_FEE_MUST_BE_GREATER_THAN_0");
132: require(msg.value <= getMaxL1MsgFee(), "MAX_L1_MSG_FEE_EXCEEDED");
....
141: }
​
​

CODED POC

As shown in the POC below, the user deposits successfully with 1 wei of gas

  • Add the test case below to the Bridge.t.solfile and then run forge test --mt test_depositTokenERC721Nogas -vv

function test_depositTokenERC721Nogas() public {
IERC721MintRangeFree(erc721C1).mintRangeFree(alice, 0, 10);
​
uint256[] memory ids = new uint256[](2);
ids[0] = 0;
ids[1] = 9;
​
uint256 salt = 0x1;
snaddress to = Cairo.snaddressWrap(0x1);
​
vm.startPrank(alice);
IERC721(erc721C1).setApprovalForAll(address(bridge), true);
uint gasBegin = gasleft();
IStarklane(bridge).depositTokens{value: 1}( // @audit deposit is made with 1 wei of gas
salt,
address(erc721C1),
to,
ids,
false
);
uint gasEnd = gasleft();
​
emit log_named_uint("Gas used for deposit call: ", gasBegin - gasEnd);
vm.stopPrank();
​
uint gasUsed = gasBegin - gasEnd;
​
assertEq(gasUsed, 230681); // @audit tx uses over 200k gas
​
}

Impact

User make deposit without paying the actual amount of gas used in the transaction

Tools Used

Foundry test

Recommendations

Implement checks directly in the Bridge::depositTokens(...) function on L1 to ensure that the correct gas fees are paid whenever deposits are initiated.
In fact, a minimum gas fee should be implemented.

Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-not-enough-fee-can-block-NFT

Impact: Medium/High. Need an admin to start a cancellation and wait for 5 days once done. DoS > 5 days. Likelyhood: Low. Everytime a wallet/or a user do not send enough gas

Support

FAQs

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