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

[M-1] Unchecked Return Value in `Bridge.sol`: `_cancelRequest()` Function Fails to Validate Result from `_withdrawFromEscrow`

calc1f7r

Summary

In the internal _cancelRequest function of the bridge.sol contract, there is a call to _withdrawFromEscrow, a function responsible for returning NFTs from escrow to their rightful owner. However, the return value of this function remains unchecked, which could lead to inconsistencies if the withdrawal fails.

Vulnerability Details

When an NFT is bridged, there might be cases where the user wishes to cancel the request. This is done through the cancelRequest(uint256[] memory payload, uint256 nonce) function, which in turn calls the internal _cancelRequest() function. Within this function, the _withdrawFromEscrow() function is called to transfer the NFTs back to the owner. This function returns a boolean value true upon successful completion, but this return value is not checked, which it should be.

Impact

Medium: If _withdrawFromEscrow fails, the failure will go unnoticed. This means that the tokens might not be withdrawn from escrow as expected, but the function will continue executing as if everything went fine.

Recommendations

function _cancelRequest(Request memory req) internal {
uint256 header = felt252.unwrap(req.header);
CollectionType ctype = Protocol.collectionTypeFromHeader(header);
address collectionL1 = req.collectionL1;
​
bool success = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
require(success, "Withdraw from escrow failed");
}
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

invalid-_withdrawFromEscrow-result-not-checked

To cancel a message, it has to be sent to the Starknet Core, otherwise it reverts. Therefore, to cancel a request, a token will always be escrowed. There is no impact here because the described case will never happen, that’s why check that boolean is not useful.

Support

FAQs

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