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

Any user can cancel request belonging to other users.

tigerfrake

Summary

From the previous audit, the auditor argues that the startRequestCancellation() function has the modifier onlyOwner meaning that only the owner is able to initiate cancellation while any user must be able to do that. He therefore reccomends that the modifier be removed from cancelRequest().

However, now the cancelRequest() function lacks access control, allowing any user to cancel any request. This can lead to unauthorized cancellations and potential abuse.

Vulnerability Details

Here is cancelRequest():

function cancelRequest(
uint256[] memory payload,
uint256 nonce
) external {
IStarknetMessaging(_starknetCoreAddress).cancelL1ToL2Message(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload,
nonce
);
Request memory req = Protocol.requestDeserialize(payload, 0);
>> _cancelRequest(req);
emit CancelRequestCompleted(req.hash, block.timestamp);
}

It internally calls _cancelRequest() as shown here which handles the internal logic for canceling a request such as:

  1. Extracts the request details from the serialized payload.

  2. Moves tokens back to the owner's possession if they were held in escrow.

// @audit No access control
function _cancelRequest(Request memory req) internal {
uint256 header = felt252.unwrap(req.header);
CollectionType ctype = Protocol.collectionTypeFromHeader(header);
address collectionL1 = req.collectionL1;
for (uint256 i = 0; i < req.tokenIds.length; i++) {
uint256 id = req.tokenIds[i];
_withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
}
}

The cancelRequest() function is vulnerable to unauthorized cancellations because it does not have any access control.

Root Cause: Missing ownership verification for the request being canceled.

Impact

Missing ownership verification for the request can lead to unauthorized cancellations and potential abuse.

Tools Used

Manual Review

Recommendations

Verify Ownership: Ensure that only the owner of a request can cancel it.

function cancelRequest(
uint256[] memory payload,
uint256 nonce
) external {
IStarknetMessaging(_starknetCoreAddress).cancelL1ToL2Message(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload,
nonce
);
Request memory req = Protocol.requestDeserialize(payload, 0);
+ require(msg.sender == req.ownerL1, "Unauthorised Access");
_cancelRequest(req);
emit CancelRequestCompleted(req.hash, block.timestamp);
}
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid-cancel-callable-by-anyone

The process to cancel a message is detailed here: https://docs.starknet.io/architecture-and-concepts/network-architecture/messaging-mechanism/#l2-l1_message_cancellation Since `startRequestCancellation` has the `onlyOwner`, only the owner can begin that process.

Support

FAQs

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