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

Unrestricted Contract Deployment via Public Library Functions

juggernaut63

Summary

The Deployer library contains public functions that allow anyone to deploy new ERC721 and ERC1155 contracts without any access control. This can lead to unauthorized contract deployments, potentially causing significant financial loss due to high gas costs and misuse of deployed contracts.

Vulnerability Details

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/token/Deployer.sol#L1-L62

1.Visibility Issue:

  • The functions deployERC721Bridgeable and deployERC1155Bridgeable are marked as public in the Deployer library, allowing them to be called by any external account.

Deployer.deployERC721Bridgeable("MyToken", "MTK");

2.Initialization Issue:

  • The initialization of the contracts uses abi.encode within abi.encodeWithSelector, which might not correctly initialize the contracts.

bytes memory dataInit = abi.encodeWithSelector(
ERC721Bridgeable.initialize.selector,
abi.encode(name, symbol)
);

3.Lack of Access Control:

  • There is no access control mechanism to restrict who can call the deployment functions.

bytes memory dataInit = abi.encodeWithSelector(
ERC721Bridgeable.initialize.selector,
abi.encode(name, symbol)
);

Impact

  • Unrestricted deployments can result in significant gas costs being incurred by the contract owner or the network.

  • Excessive deployments can lead to network congestion and resource exhaustion.

  • Unauthorized contracts might be used for malicious purposes, such as phishing or fraudulent activities.

Tools Used

Manual review

Recommendations

  • Change function visibility to internal.

function deployERC721Bridgeable(
string memory name,
string memory symbol
)
internal
returns (address)
{
// ...
}
​
function deployERC1155Bridgeable(
string memory uri
)
internal
returns (address)
{
// ...
}

  • Make sure the initialization parameters are sent without abi.encode inside abi.encodeWithSelector.

bytes memory dataInit = abi.encodeWithSelector(
ERC721Bridgeable.initialize.selector,
name,
symbol
);
​
bytes memory dataInit = abi.encodeWithSelector(
ERC1155Bridgeable.initialize.selector,
uri
);

  • Add access control using onlyOwner and turn the library into a contract.

// SPDX-License-Identifier: MIT
​
pragma solidity ^0.8.0;
​
import "openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import "openzeppelin-contracts/contracts/access/Ownable.sol";
import "./ERC721Bridgeable.sol";
import "./ERC1155Bridgeable.sol";
​
/**
@title Collection contract deployer.
*/
contract Deployer is Ownable {
​
/**
@notice Deploys a UUPS proxied ERC721 contract.
​
@param name Descriptive name for the token collection.
@param symbol Abbreviated name for the token collection.
​
@return Address of the proxy.
*/
function deployERC721Bridgeable(
string memory name,
string memory symbol
)
public
onlyOwner
returns (address)
{
address impl = address(new ERC721Bridgeable());
bytes memory dataInit = abi.encodeWithSelector(
ERC721Bridgeable.initialize.selector,
name,
symbol
);
​
return address(new ERC1967Proxy(impl, dataInit));
}
​
/**
@notice Deploys a UUPS proxied ERC1155 contract.
​
@param uri URI with token id placeholder.
​
@return Address of the proxy.
*/
function deployERC1155Bridgeable(
string memory uri
)
public
onlyOwner
returns (address)
{
address impl = address(new ERC1155Bridgeable());
bytes memory dataInit = abi.encodeWithSelector(
ERC1155Bridgeable.initialize.selector,
uri
);
​
return address(new ERC1967Proxy(impl, dataInit));
}
}
Updates

Lead Judging Commences

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

Support

FAQs

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