Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/factory/CashFactory.sol#L75-L85

Vulnerability details

Impact

/**
 * @dev This function will deploy an upgradable instance of Cash
 *
 * @param name   The name of the token we want to deploy.
 * @param ticker The ticker for the token we want to deploy.
 *
 * @return address The address of the proxy contract.
 */
function deployCash(string memory name, string memory ticker) public {
  cashImplementation = new Cash(name, ticker);
  cashProxyAdmin = new ProxyAdmin();
  cashProxyAdmin.initialize();
  cashProxy = new TokenProxy(address(cashImplementation), address(cashProxyAdmin));
}

It is not clear from this code how the guardian address is protected from malicious actors, and anyone could potentially call the deployCash function and deploy a new version of the Cash contract.

This bug could have a significant impact on the security and integrity of the Cash contract and the funds that are managed by it.

Without proper access controls in place, any malicious actor could potentially call the deployCash function and deploy a new version of the Cash contract. This could potentially allow the attacker to steal funds from the contract, or make arbitrary changes to its logic and behavior, potentially leading to a loss of funds or other negative consequences.

Additionally, if the attacker is able to deploy a new version of the Cash contract, they could potentially use the new contract to conduct unauthorized transactions, such as minting new tokens or pausing the contract. This could potentially lead to a loss of funds or other negative consequences.

Furthermore, this bug could potentially enable the attacker to gain control of the guardian address, which is intended to be a multisignature address. This could allow the attacker to gain control of the Cash contract and the funds that are managed by it.

Proof of Concept

this bug would involve creating an instance of the "CashFactory" contract and calling the "deployCash" function without any proper access controls in place.

Create an instance of the "CashFactory" contract by deploying it to the blockchain and passing in an arbitrary address as the guardian address.

Call the "deployCash" function from any address, passing in a new name and ticker for the Cash contract.

Observe that a new version of the Cash contract is deployed and the guardian address is set to the address passed in when creating the "CashFactory" contract instance.

This demonstrates that any address can call the "deployCash" function and deploy a new version of the Cash contract, which could potentially lead to a loss of funds or other negative consequences

Tools Used

Slither, VSCODE, Nodejs

Recommended Mitigation Steps

to achieve this is to use a modifier to check the caller's address before executing the deployCash function. A modifier can be defined in the contract to check if the caller's address is equal to the guardian address set in the constructor of the contract. If the caller's address does not match the guardian address, the function will not execute and an error message can be returned

modifier onlyGuardian {
    require(msg.sender == guardian, "Only guardian can call this function");
    _;
}

function deployCash(string memory name, string memory ticker) public onlyGuardian {
  cashImplementation = new Cash(name, ticker);
  cashProxyAdmin = new ProxyAdmin();
  cashProxyAdmin.initialize();
  cashProxy = new TokenProxy(address(cashImplementation), address(cashProxyAdmin));
}

Alternatively, you can use role-based access control libraries like OpenZeppelin's AccessControl or use smart contract security frameworks like OpenZeppelin's upgradeable contract patterns, which handle the authorization checks, and deployment of the new contract

code-423n4 / 2023-01-ondo-findings