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

`Starklane::startRequestCancellation` is marked `OnlyOwner`, no user would be able to request for cancellation.

unRekt

Summary

The Starklane::startRequestCancellation is marked as OnlyOwner which would allow only owner of the contract to request for cancellation, but any user should be able to do it.

Vulnerability Details

The same issue was reported in the previous audit as shared by the protocol, and it was marked as fixed but it still remains the same. The function is meant to request for cancellation by an user, which would not be possible as it is marked onlyOwner

The use of Starklane::startRequestCancellation function is to initiate a request for cancellation after a certain delay the user can call Starklane::cancelRequest to cancel the request. These two functions work together by calling one after the other.

startRequestCancellation function

The startRequestCancellation function calls startL1ToL2MessageCancellation of starknet.

/**
Starts the cancellation of an L1 to L2 message.
A message can be canceled messageCancellationDelay() seconds after this function is called.
​
Note: This function may only be called for a message that is currently pending and the caller
must be the sender of the that message.
*/
function startL1ToL2MessageCancellation(
uint256 toAddress,
uint256 selector,
uint256[] calldata payload,
uint256 nonce
) external returns (bytes32);

From the comment above startL1ToL2MessageCancellation

caller must be the sender of the that message.

Hence, the function can be called only by the one who sent it

cancelRequest function

The cancelRequest function calls cancelL1ToL2Message of starknet.

/**
Cancels an L1 to L2 message, this function should be called at least
messageCancellationDelay() seconds after the call to startL1ToL2MessageCancellation().
A message may only be cancelled by its sender.
If the message is missing, the call will revert.
​
Note that the message fee is not refunded.
*/
function cancelL1ToL2Message(
uint256 toAddress,
uint256 selector,
uint256[] calldata payload,
uint256 nonce
) external returns (bytes32);

From the comments above cancelL1ToL2Message

A message may only be cancelled by its sender.

Hence only the sender of the message will be able to cancel the request.

Now, if startRequestCancellation request is marked as onlyOwner that means only owner should be able to cancel the messages, then cancelRequest should also be marked as onlyOwner because no one other than the owner will be able to call this function.

Impact

In short, caller of both the function should be one user who sent the message. But having different access control in both function breaks the functionality.

Tools Used

Manual review

Recommendations

Remove onlyOwner modifier from startRequestCancellation function.

Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
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.