hunter_w3b - Lack of Access Control in deployDistributor Function

[sherlock-admin2]

hunter_w3b

medium

Lack of Access Control in deployDistributor Function

Summary

The deployDistributor function in the PerAddressContinuousVestingMerkleDistributorFactory and PerAddressContinuousVestingMerkleDistributorFactory contract lacks access control, allowing anyone to deploy a new distributor. This function should be restricted to authorized entities to prevent unauthorized deployments.

Vulnerability Detail

The deployDistributor function is publicly accessible, meaning any user can call this function and deploy a new PerAddressContinuousVestingMerkleDistributor contract. This function should ideally be restricted to the contract owner or an authorized entity to prevent unauthorized deployments.

function deployDistributor(
    IERC20 _token,
    uint256 _total,
    string memory _uri,
    bytes32 _merkleRoot,
    uint160 _maxDelayTime,
    address _owner,
    uint256 _nonce
) public returns (PerAddressContinuousVestingMerkleDistributor distributor) {
    bytes32 salt = _getSalt(
        _token,
        _total,
        _uri,
        _merkleRoot,
        _maxDelayTime,
        _owner,
        _nonce
    );

    distributor = PerAddressContinuousVestingMerkleDistributor(Clones.cloneDeterministic(i_implementation, salt));
    distributors.push(address(distributor));

    emit DistributorDeployed(address(distributor));

    distributor.initialize(_token, _total, _uri, _merkleRoot, _maxDelayTime, _owner);

    return distributor;
}

Impact

Unintended Deployments:

• Unauthorized entities can deploy distributors, leading to a potentially large number of unnecessary distributor contracts.
• Increased gas costs for the contract owner due to the additional storage used when new distributors are added to the distributors array.

Predictable Addresses:

• Malicious actors can precompute the addresses of the new distributors and deploy contracts with malicious intent.

Code Snippet

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/claim/factory/PerAddressContinuousVestingMerkleDistributorFactory.sol#L39-L68

https://github.com/sherlock-audit/2024-05-tokensoft-distributor-contracts-update/blob/main/contracts/packages/hardhat/contracts/claim/factory/PerAddressContinuousVestingMerkleDistributorFactory.sol#L39-L68

Tool used

Manual Review

Recommendation

Implement access control using OpenZeppelin's Ownable contract to restrict the deployDistributor function to the contract owner. This will prevent unauthorized entities from deploying new distributors.

[hunter-w3b]

Escalate

Admin function Lack of Access Control

[sherlock-admin3]

Escalate

Admin function Lack of Access Control

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page, in the Sherlock webapp.

[MxAxM]

Escalate

Admin function Lack of Access Control

There is no clear impact, provide more information if you think it's valid

[hunter-w3b]

@MxAxM

The function predictDistributorAddress is public, allowing us to predict the Distributor Address.

The deployDistributor function uses the cloneDeterministic function. The actual problem arises from this. This function uses the create2 opcode and a salt to deterministically deploy the clone. Using the same implementation and salt multiple time will cause a revert.

Malicious actors can precompute the addresses of new distributors before they are deployed. This predictability can be exploited in several ways:

1. Front-Running Attacks: An attacker can deploy a malicious contract at the predicted address, potentially intercepting transactions or executing unintended operations.
2. Address Squatting: Malicious actors can deploy dummy contracts to predicted addresses, causing DOS preventing legitimate deployments.

As a result, the admin cannot deploy the distributor because it will revert. An attacker can precompute the address of a new distributor and deploy a malicious contract at that address before the legitimate user.

distributor = PerAddressContinuousVestingMerkleDistributor(Clones.cloneDeterministic(i_implementation, salt));
distributors.push(address(distributor));

Clones.cloneDeterministic

/**
 * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation`.
 *
 * This function uses the create2 opcode and a `salt` to deterministically deploy
 * the clone. Using the same `implementation` and `salt` multiple times will revert, since
 * the clones cannot be deployed twice at the same address.
 */
function cloneDeterministic(address implementation, bytes32 salt) internal returns (address instance) {
    /// @solidity memory-safe-assembly
    assembly {
        // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes
        // of the `implementation` address with the bytecode before the address.
        mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
        // Packs the remaining 17 bytes of `implementation` with the bytecode after the address.
        mstore(0x20, or(shl(0x78, implementation), 0x5af43d82803e903d91602b57fd5bf3))
        instance := create2(0, 0x09, 0x37, salt)
    }
    require(instance != address(0), "ERC1167: create2 failed");
}

[hunter-w3b]

[MxAxM]

There is no benefit for attacker, also it doesn't block deploying new distributors, owner can simply use a different nonce for deployment

[hunter-w3b]

@MxAxM

If an attacker successfully deploys a malicious or dummy contract at the predicted address, the legitimate owner would need to use a different nonce for deployment. This process can lead to increased gas costs for the owner, as they may need to iterate through multiple nonce values until they find an available address.

The attacker can precompute the address and deploy a malicious contract before the owner, preventing the owner from deploying the distributor at the intended address.

[jkoppel]

That's not an attack. That's actually saving gas costs for the owner. There is no way to deploy to the owner's predicted address without also transferring control to said owner.