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

Improper Handling of Invalid Inputs `deployERC721Bridgeable` and `deployERC1155Bridgeable` Functions in Deployer.sol

heim

Summary

During fuzz testing of the deployment functions in Deployer.sol, vulnerabilities were identified where the contract does not properly handle invalid inputs such as empty strings. This report details these issues, the expected behavior, and recommendations for addressing them.

Vulnerability Details

The fuzz tests revealed that the deployERC721 and deployERC1155 functions did not adequately check for invalid inputs, like empty strings. When such inputs were provided, the functions failed to revert, leading to the deployment of contracts with incorrect parameters.

Proof of Concept (PoC) – Demonstration with Fuzz

pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "./DeploymentManager.sol";
import "../../src/token/ERC721Bridgeable.sol";
import "../../../src/token/ERC1155Bridgeable.sol";
​
contract TestDeploymentManager is Test {
DeploymentManager deploymentManager;
​
function setUp() public {
deploymentManager = new DeploymentManager();
}
​
// Test to fuzz input for ERC721 deployment and check proper handling of invalid inputs
function testAliceDeploymentFuzz(string memory name, string memory symbol) public {
// Invalid inputs: Expect revert when any of the inputs are empty
if (bytes(name).length == 0 || bytes(symbol).length == 0) {
vm.expectRevert("Name or symbol cannot be empty");
deploymentManager.deployERC721(name, symbol);
return;
}
​
// Valid inputs: Deploy contract and check deployment
deploymentManager.deployERC721(name, symbol);
address aliceLastDeployed = deploymentManager.lastDeployed();
assertTrue(aliceLastDeployed != address(0), "Alice deployment failed with fuzzed inputs");
​
// Verify that the deployed contract has correct parameters
ERC721Bridgeable deployedContract = ERC721Bridgeable(payable(aliceLastDeployed));
assertEq(deployedContract.name(), name, "ERC721 Name did not match");
assertEq(deployedContract.symbol(), symbol, "ERC721 Symbol did not match");
}
​
// Test to fuzz input for ERC1155 deployment and check proper handling of invalid URI
function testBobDeploymentFuzz(string memory uri) public {
// Invalid input: Expect revert when URI is empty
if (bytes(uri).length == 0) {
vm.expectRevert("URI cannot be empty");
deploymentManager.deployERC1155(uri);
return;
}
​
// Valid input: Deploy contract and check deployment
deploymentManager.deployERC1155(uri);
address bobLastDeployed = deploymentManager.lastDeployed();
assertTrue(bobLastDeployed != address(0), "Bob deployment failed with fuzzed input");
​
// Verify that the deployed contract has correct URI
ERC1155Bridgeable deployedContract = ERC1155Bridgeable(payable(bobLastDeployed));
assertEq(deployedContract.uri(0), uri, "ERC1155 URI did not match");
}
​
// Negative test cases to ensure contract reverts on invalid inputs
function testNegativeCases() public {
// Test with empty name and symbol for ERC721
vm.expectRevert("Name or symbol cannot be empty");
deploymentManager.deployERC721("", "");
​
// Test with empty URI for ERC1155
vm.expectRevert("URI cannot be empty");
deploymentManager.deployERC1155("");
}
}

Impact

Severity: Medium

The improper handling of invalid inputs can lead to the deployment of contracts with incorrect parameters or metadata. Although the contract itself may not be critically compromised, this can lead to user confusion, mismanagement of contract state, and potential issues with downstream processes relying on correct contract data.

Tools Used

  • Manual review

  • Fuzz testing in foundry

Recommendations

Check all Inputs:

  • Ensure that all input parameters in the deployment functions are non-empty.

  • Use the require statement to enforce this validation:

+ require(bytes(name).length > 0, "Name cannot be empty");
+ require(bytes(symbol).length > 0, "Symbol cannot be + empty");
+ require(bytes(uri).length > 0, "URI cannot be empty");
Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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