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

Unchecked return value from `Bridge::_cancelRequest(...)` on L1

audinarey

Summary

The Bridge::_cancelRequest()function is used to cancel request on L1, however the the function does not check if the _withdrawFromEscrow(...)call is successful and the asset is returned to the owner form the escrow

Vulnerability Details

The problem is that if the return of the asset in the _withdrawFromEscrow(...)call fails, the asset will not be returned to the owner and the _escrow[collection][id] = address(0x0)is set to zero and the user has forfeiteed their NFT.

File: Bridge.sol
258: function _cancelRequest(Request memory req) internal {
SNIP .......
262: for (uint256 i = 0; i < req.tokenIds.length; i++) {
263: uint256 id = req.tokenIds[i]; // @audit LOW unchecked withdrawal return value
264: @> _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id); // @audit returns true or false without actually wiithdrawing
265: }
266: }
​
​
​
File: Escrow.sol
63: function _withdrawFromEscrow(
64: CollectionType collectionType,
65: address collection,
66: address to,
67: uint256 id
68: )
69: internal
70: returns (bool)
71: {
72: if (!_isEscrowed(collection, id)) {
73: return false;
74: }
75:
76: address from = address(this);
SNIP ......
85:
86: @> _escrow[collection][id] = address(0x0);
87:
88: return true;
89: }
​

Impact

Possible loss of user funds

Tools Used

Manual review

Recommendations

Modify the _cancelRequest(...) function as shown below

File: Bridge.sol
258: function _cancelRequest(Request memory req) internal {
SNIP .......
262: for (uint256 i = 0; i < req.tokenIds.length; i++) {
263: uint256 id = req.tokenIds[i]; // @audit LOW unchecked withdrawal return value
-264: _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
+264: require(_withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id), "Failed withdrawal"); // @audit returns true or false without actually wiithdrawing
265: }
266: }
Updates

Lead Judging Commences

n0kto Lead Judge 10 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.