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

`_callBaseUri` will return true even in case of `_baseUri` is empty String ""

0xaman

Summary

The _callBaseUri function performs a staticcall to the ERC721 contract to check if it implements either the _baseUri() or baseUri function and returns a valid string(URI). It will return true if the function call is successful, but it will also return true if the function returns an empty string "".

Vulnerability Details

Inside _callBaseUri, the Ark Project code will use staticcall to check if the ERC721 contract implements either _baseUri or baseUri. The staticcall will not revert if the ERC721 contract does not implement the called functions.

function _callBaseUri(
address collection
)
internal
view
returns (bool, string memory)
{
bool success;
uint256 returnSize;
uint256 returnValue;
bytes memory ret;
bytes[2] memory encodedSignatures = [abi.encodeWithSignature("_baseUri()"), abi.encodeWithSignature("baseUri()")]; // @audit : sig would be baseURI instead of baseUri as Openzeppline implemnts baseUri
​
for (uint256 i = 0; i < 2; i++) {
bytes memory encodedParams = encodedSignatures[i];
assembly {
success := staticcall(gas(), collection, add(encodedParams, 0x20), mload(encodedParams), 0x00, 0x20)
if success {
returnSize := returndatasize()
returnValue := mload(0x00)
ret := mload(0x40)
mstore(ret, returnSize)
returndatacopy(add(ret, 0x20), 0, returnSize)
// update free memory pointer
mstore(0x40, add(add(ret, 0x20), add(returnSize, 0x20)))
}
}
@1>-- if (success && returnSize >= 0x20 && returnValue > 0) {
@>2-- return (true, abi.decode(ret, (string)));
}
}
return (false, "");
}

One thing to note is that the default implementation of _baseURI will return an empty string (""). If such an ERC721 collection is added, all its NFTs metadata will be lost on Layer 2 Starknet due to incorrect checks @1>-- and @>2--.

POC:
Add following simple code to ethereum/test/TestUri.t.sol folder:

// SPDX-License-Identifier: UNLICENSED
​
pragma solidity 0.8.22;
​
import "forge-std/Test.sol";
​
contract ERC721Contract {
constructor() {}
​
function baseUri() external returns (string memory) {
return ""; // it will return empty string
}
}
​
contract TestUri is Test {
ERC721Contract public contractERC721;
function setUp() external {
contractERC721 = new ERC721Contract();
}
// code of _callBaseUri add here for testing
function _callBaseUri() internal returns (bool, string memory) {
ERC721Contract _erc721TestContract = contractERC721;
bool success;
uint256 returnSize;
uint256 returnValue;
bytes memory ret;
bytes[2] memory encodedSignatures = [
abi.encodeWithSignature("_baseUri()"),
abi.encodeWithSignature("baseUri()")
];
​
for (uint256 i = 0; i < 2; i++) {
bytes memory encodedParams = encodedSignatures[i];
assembly {
success := staticcall(
gas(),
_erc721TestContract,
add(encodedParams, 0x20),
mload(encodedParams),
0x00,
0x20
)
if success {
returnSize := returndatasize()
returnValue := mload(0x00)
ret := mload(0x40)
mstore(ret, returnSize)
returndatacopy(add(ret, 0x20), 0, returnSize)
// update free memory pointer
mstore(0x40, add(add(ret, 0x20), add(returnSize, 0x20)))
}
}
if (success && returnSize >= 0x20 && returnValue > 0) {
return (true, abi.decode(ret, (string)));
}
}
return (false, "");
}
​
function test_no_uri_true() external {
(bool success, string memory val) = checkCase();
assertEq(val, "");
assertEq(success, true);
}
}
​

run with command : forge test --mt test_no_uri_true -vvv

Impact

The _callBaseUri function will return true even if the baseUri function returns an empty string (""). This can result in the loss of NFTs metadata on Layer 2 due to the absence of a URI attached to the NFT.

Tools Used

Manual Review , Foundry

Recommendations

Check that the value returned from the low-level call is a valid string and that its length is greater than 0. One potential fix would be to add the following check when success is true:

diff --git a/apps/blockchain/ethereum/src/token/TokenUtil.sol b/apps/blockchain/ethereum/src/token/TokenUtil.sol
index 41cc17d..dbb6638 100644
--- a/apps/blockchain/ethereum/src/token/TokenUtil.sol
+++ b/apps/blockchain/ethereum/src/token/TokenUtil.sol
@@ -164,7 +164,9 @@ library TokenUtil {
}
}
if (success && returnSize >= 0x20 && returnValue > 0) {
- return (true, abi.decode(ret, (string)));
+ string memory s = abi.decode(ret, (string));
+ if(bytes(s).length > 0 ) return (true , s);
+ else return (false, "");
}
}
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-empty-baseURI-do-not-use-tokenURI

Likelyhood: Low, baseURI is not in the ERC721 standard. Impact: Low, function won’t work when it has to. But URI can be set later.

Appeal created

0xaman Submitter
9 months ago
n0kto Lead Judge
9 months ago
n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-empty-baseURI-do-not-use-tokenURI

Likelyhood: Low, baseURI is not in the ERC721 standard. Impact: Low, function won’t work when it has to. But URI can be set later.

Support

FAQs

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