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

attackers could withdraw other users' tokens

zhanmingjing

Summary

Attackers could withdraw other users' tokens

Vulnerability Details

When users withdraw tokens with Bridge.sol:withdrawTokens(), provide parameter req, Escrow.sol:_withdrawFromEscrow() will be called.

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Bridge.sol#L201

​
bool wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Escrow.sol#L72-L74

In Escrow.sol：_withdrawFromEscrow（）， only theck if collection and id is Escrowed, it is not enough, should check if collection and id is Escrowed by "address to". Then attackers could input other users'id in param req, and call Bridge.sol:withdrawTokens(), to withdraw other users' tokens.

function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
returns (bool)
{
if (!_isEscrowed(collection, id)) {
return false;
}
​
address from = address(this);
​
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id);
} else {
// TODO:
// Check here if the token supply is currently 0.
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
​
_escrow[collection][id] = address(0x0);
​
return true;
}

Impact

Attackers could withdraw other users' tokens.

Tools Used

manually reviewed.

Recommendations

function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
returns (bool)
{
if (_escrow[collection][id] != to) {
return false;
}
​
address from = address(this);
​
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id);
} else {
// TODO:
// Check here if the token supply is currently 0.
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
​
_escrow[collection][id] = address(0x0);
​
return true;
}
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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