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 11 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
11 months ago
n0kto Lead Judge
11 months ago
n0kto Lead Judge 11 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.