Factory lacks existence checks when

[ptisserand]

From Cyfrin/2024-07-ark-project#2

[Low-1] Factory lacks existence checks

When utilizing a factory pattern to deploy contracts based on user input, it's crucial to ensure that the contract isn't being deployed at an address already in use. Deploying to an existing address can lead to unintentional overwrites or unexpected behavior. To prevent this, the factory contract should maintain a mapping or a list of deployed contract addresses. Before creating a new contract, the factory should check if the intended address is already in the list. If it is, the creation should be halted or rerouted. Implementing such checks minimizes risks, ensuring that contracts are only deployed to fresh, uncontaminated addresses, preserving the integrity of the protocol.

function deployERC721Bridgeable(
    string memory name,
    string memory symbol
)
    public
    returns (address)
{
    address impl = address(new ERC721Bridgeable()); // <= FOUND
    
    bytes memory dataInit = abi.encodeWithSelector(
        ERC721Bridgeable.initialize.selector,
        abi.encode(name, symbol)
   
    return address(new ERC1967Proxy(impl, dataInit));
}

[ikemHood]

I would be happy to take this up by tomorrow 💫

[mitarumi]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have years of expertise in backend development with python, solidity and cairo

How I plan on tackling this issue

I'd create a mapping of deployed contract addresses and include a check to confirm if the contract has been deployed at an address on the list. If so the contract deployment should be stopped

[aji70]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

i'm a solidity and cairo smart contract developer with over 2 years experience and believe i have the skill set for the task and i am also very good with smart contract testing

How I plan on tackling this issue

i would implement a mapping that would track the addresses of the already deployed contract, and would create a check for the intended address for any new contract to be deployed and compare it against the already mapped addresses if there is a clash the address should be regenerated for the intended new contract else it should be deployed

[ryzen-xp]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I'm a Solidity adn cairo developer specializing in NFT marketplaces and decentralized apps, with experience in multi-token support and integrating blockchain protocols. My work on projects like Worldcoin-Bridge-Linea equips me to handle tasks like adding ERC-20 etc support efficiently.

How I plan on tackling this issue

To address the lack of existence checks in the factory:

1. Maintain a mapping** of deployed contract addresses in the factory to track existing contracts.
2. Before deploying a new contract in deployERC721Bridgeable, check if the intended address is already in the mapping.
3. If it exists, halt the deployment** and return an error or redirect the process.
4. Update the mapping after a successful deployment to include the new contract address.

[0xDeon]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Good day again ArkProject, my second application to your repo, lol. My name is Deon and I'd like to apply formally for the task presented. I am a Web and blockchain engineer with a passion for building user interfaces and Dapps that deliver meaningful experiences. With a background in Computer Science (BSc) and hands-on experience. If given the chance to contribute this will be my second official contribution via onlydust and I'm confident in my ability to deliver on the feature you're looking for.

How I plan on tackling this issue

Understand the Factory Deployment Logic:
I will review the factory contract to identify how user input determines deployment addresses and ensure that the logic is clear for tracking contract creation.

Use a Mapping to Track Deployed Contracts:
I will implement a mapping in the factory contract to store addresses where contracts have been deployed. Before deploying a new contract, the factory will check this mapping to ensure the address is not already in use.

Check Address Before Deployment:
I will add logic to verify whether an address is already in the mapping. If the address is already used, the contract creation will be halted or redirected to a new address, preventing overwrites.

Write Unit Tests for Validation:
I will write unit tests to validate that the mapping correctly tracks deployed contracts and that the factory only deploys to unused addresses. These tests will include both successful deployments and scenarios where deployment is blocked due to an existing contract.

Comprehensive Coverage:
I will ensure that all deployment scenarios are tested, including edge cases, to confirm the integrity of the factory contract's address-checking mechanism.

With this strategic approach, I hope prevent overwriting existing contracts, ensuring secure and reliable contract deployment.

[raizo07]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hello I'll like to be assigned to work on this.

I'm a software Dev with over four years experience and have worked on a couple of projects here.

[saimeunt]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have experience in Solidity/Cairo Smart Contract programming.

How I plan on tackling this issue

First I will carefully study the audit report to understand the implications of the security issue.
Then I will fix the issue by maintaining a list of already deployed addresses in a mapping stored as a contract member of the bridge.
I will add tests to make sure we have coverage on this security fix.

[Joewizy]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hi my name is Joseph Gimba a computer engineer graduate that has passion for web3 and blockchain development, I have experience in writing smart contracts as well as properly testing them using conventional testing methods, Invariant testing, fuzz testing and also use tools like SLITHER as well to make sure my system is robust , secured and efficient.

How I plan on tackling this issue

I would approach this problem firstly by creating a mapping(address => bool) public checkAddress; to track whether a specific contract address has already been deployed. Additionally, I would create a dynamic array, address[] public deployedAddresses;, to store all deployed contract addresses.

Before deploying a new contract with address impl = address(new ERC721Bridgeable());, I would add a check to ensure the contract hasn't already been deployed: which would be: if(checkAddress[impl]){
revert ERC721Bridgeable__AddressTaken(); // custom error if true;
checkAddress[impl] = true;
deployedAddresses.push(impl); // save address to array

By using this method when the function is called it first check if the address has already be deployed if is false it marks as true and save the deployed address to they deployedAddresses list.

We can also emit an Event when the contract is deployed to be able to track it on the frontend for future purposes.

[Luluameh]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have experience in Solidity and contract deployment patterns. I’ve worked with factory contracts and understand the importance of preventing address collisions during deployment to maintain protocol integrity.

How I plan on tackling this issue

I’d modify the factory to maintain a mapping of deployed addresses and implement a check before deploying new contracts. If an address is in use, the deployment would be halted or rerouted to avoid overwriting existing contracts.

[DevPelz]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have extensive experience as a security researcher and Solidity developer, specializing in smart contract auditing and development. My expertise includes building and auditing contracts across various platforms, ensuring their security and functionality through deep testing, fuzzing, and vulnerability assessment techniques. I’ve worked on both private and public audits, including high-profile projects like Worldcoin Bridge Linea and ERC20 pools, where I've uncovered numerous vulnerabilities.

My proficiency with security tools like Foundry and Echidna for fuzz testing and property writing ensures robust contract assessments. Furthermore, I've implemented the factory pattern in contract deployments, allowing me to recognize and mitigate risks related to address collisions and other vulnerabilities. I am skilled at ensuring that contracts are deployed securely and aren’t susceptible to risks like re-deployment to already-in-use addresses.

How I plan on tackling this issue

Understand the Factory Pattern: I would first evaluate the factory pattern you're using to deploy contracts. I'd ensure the factory keeps track of all deployed addresses and checks whether an address is already in use.

Pre-Deployment Address Checking:

Implement checks to see if the contract deployment address is already taken.
Use either create2 for deterministic deployment or maintain a mapping of deployed contracts to check if an address is in use before deploying a new one.
Ensure the factory reverts or reroutes if a contract is being deployed to an occupied address.
Security Measures:

Integrate access controls in the factory to restrict who can deploy contracts.
Run fuzzing tests and other security audits to identify potential vulnerabilities or logical flaws in the contract deployment process.
Test Cases:

Write comprehensive test cases using Foundry or Echidna to simulate various deployment scenarios, such as deploying to addresses already in use, preventing unauthorized deployments, and handling invalid user inputs.

[DevPelz]

Hi @ptisserand is there a way i can contact you on telegram. I have a few things to let you know

[ptisserand]

Hi @ptisserand is there a way i can contact you on telegram. I have a few things to let you know

Sure, here is the Ark Project telegram group: https://t.me/arkprojectnfts