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

_depositIntoEscrow()::Escrow.sol considers ERC1155 but depositTokens()::Bridge.sol that calls it reverts if it's an ERC1155

0xziin

Summary

Either you accept ERC1155 or you don't, but you cannot consider ERC1155 in _depositIntoEscrow()::Escrow.sol & in depositTokens()::Bridge.sol#L125-L127 but before that, early in the function #L96-98, reverting if it is an ERC1155.

_depositIntoEscrow()::Escrow.sol considers ERC1155 but depositTokens()::Bridge.sol that calls it reverts if it's an ERC1155.
Not only that, but depositTokens()::Bridge.sol itself considers ERC1155 :

...} else {
(req.uri) = TokenUtil.erc1155Metadata(collectionL1);
}

Vulnerability Details

Here we can see that depositTokens()::Bridge.sol reverts if it's an ERC1155 :

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

if (ctype == CollectionType.ERC1155) {
revert NotSupportedYetError();
}

but later on, there is a call to :
_depositIntoEscrow() from Escrow.sol and we can see that this function considers ERC1155 :

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

if (collectionType == CollectionType.ERC721) {
IERC721(collection).transferFrom(msg.sender, address(this), id);
} else {
// TODO: check the supply is exactly one.
// (this is the low level call to verify if a contract has some function).
// (but it's better to check with supported interfaces? It's 2 calls instead
// of one where we control the fail.)
//(bool success, bytes memory data) = contractAddress.call("");
IERC1155(collection).safeTransferFrom(msg.sender, address(this), id, 1, "");
}

Even here, ERC1155 is considered :

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

if (ctype == CollectionType.ERC721) {
(req.name, req.symbol, req.uri, req.tokenURIs) = TokenUtil.erc721Metadata(
collectionL1,
ids
);
} else {
(req.uri) = TokenUtil.erc1155Metadata(collectionL1);
}
​
_depositIntoEscrow(ctype, collectionL1, ids);

Impact

The protocol doesn't work like intended. It will always revert if it's an ERC1155, even though all the code to deal with ERC1155 is present in the contract.
The user won't be able to deposit ERC1155.

Tools Used

Github, VisualCode, Foundry.

Recommendations

Remove Bridge.sol#L96-L98:

if (ctype == CollectionType.ERC1155) {
revert NotSupportedYetError();
}
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Out of scope
Assigned finding tags:

invalid-ERC1155-not-in-scope

```compatibilities: Blockchains: - Ethereum/Starknet Tokens: - [ERC721](www.tokenstandard.com) ``` ``` function depositTokens( uint256 salt, address collectionL1, snaddress ownerL2, uint256[] calldata ids, bool useAutoBurn ) external payable { if (!Cairo.isFelt252(snaddress.unwrap(ownerL2))) { revert CairoWrapError(); } if (!_enabled) { revert BridgeNotEnabledError(); } CollectionType ctype = TokenUtil.detectInterface(collectionL1); if (ctype == CollectionType.ERC1155) { @> revert NotSupportedYetError(); } … } ```

Support

FAQs

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