DatingDapp
First Flight #33
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Loss of Funds due to Lack of MultiSig Deployment Verification

sauronsol

Summary

The matchRewards() function does not verify if the MultiSig contract was properly deployed before modifying states and sending funds. If the deployment fails, the funds are lost.

Vulnerability Details

In LikeRegistry.sol, the matchRewards function:

  1. Sets the user balances to zero before verifying the deployment

  2. Does not check if the MultiSig contract has been successfully deployed

  3. Lacks a rollback mechanism in case of deployment failure

function matchRewards(address from, address to) internal {
uint256 matchUserOne = userBalances[from];
uint256 matchUserTwo = userBalances[to];
userBalances[from] = 0; // Balance set to zero before verification
userBalances[to] = 0; // Balance set to zero before verification
​
uint256 totalRewards = matchUserOne + matchUserTwo;
uint256 matchingFees = (totalRewards * FIXEDFEE) / 100;
uint256 rewards = totalRewards - matchingFees;
totalFees += matchingFees;
​
// Deploy the MultiSig without verification
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to);
​
// Send ETH without verifying if the deployment was successful
(bool success,) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
}

Impact

  1. Permanent Loss of Funds : If the deployment fails, the balances are already zero and the funds remain locked in the contract

  2. No Recovery Mechanism : No way to recover the funds in case of failure

  3. Easily Exploitable : The deployment can fail for several legitimate reasons (invalid bytecode, out of gas, etc.)

Severity: HIGH - Permanent loss of funds with high probability of occurrence

Tools Used

  • Foundry for testing

  • Manual code review

  • PoC demonstrating both issues:

    • testFailedDeployment: Proves funds can be lost

    • testViolationOfCEI: Proves CEI violation

Proof of Concept

function testFailedDeployment() public {
// Setup initial state
vm.deal(alice, 2 ether);
vm.deal(bob, 2 ether);
​
// Alice likes Bob
vm.prank(alice);
likeRegistry.likeUser{value: 1 ether}(bob);
​
// Force the next deployment to fail with invalid bytecode
address predictedAddress = computeCreateAddress(
address(likeRegistry),
vm.getNonce(address(likeRegistry))
);
vm.etch(predictedAddress, hex"FF"); // Bytecode invalide
​
// Bob likes Alice, triggering matchRewards
vm.prank(bob);
vm.expectRevert();
likeRegistry.likeUser{value: 1 ether}(alice);
​
// The balances are 0 even if the deployment failed
assertEq(likeRegistry.userBalances(alice), 0);
assertEq(likeRegistry.userBalances(bob), 0);
​
// The ETH is still in the contract
assertEq(address(likeRegistry).balance, 2 ether);
​
// The contract was not deployed correctly
uint256 size;
assembly {
size := extcodesize(predictedAddress)
}
assertEq(size, 1, "Contract should have invalid bytecode size");
}

Recommendations

  1. Verify the deployment of the contract before modifying states:

function matchRewards(address from, address to) internal {
uint256 matchUserOne = userBalances[from];
uint256 matchUserTwo = userBalances[to];
uint256 totalRewards = matchUserOne + matchUserTwo;
uint256 matchingFees = (totalRewards * FIXEDFEE) / 100;
uint256 rewards = totalRewards - matchingFees;
​
// Deploy the MultiSig
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to);
​
// Verify that the contract was deployed correctly
uint256 size;
assembly {
size := extcodesize(address(multiSigWallet))
}
require(size > 0, "Contract deployment failed");
​
// Verify that the contract is a MultiSigWallet
try MultiSigWallet(address(multiSigWallet)).owner1() returns (address owner1) {
require(owner1 == from, "Invalid MultiSig deployment");
} catch {
revert("Invalid MultiSig deployment");
}
​
// Attempt the transfer
(bool success,) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
​
// Update states after successful deployment and transfer
userBalances[from] = 0;
userBalances[to] = 0;
totalFees += matchingFees;
}

  1. Implement a secure deployment pattern with a factory:

contract MultiSigFactory {
event MultiSigDeployed(address indexed multiSig, address indexed owner1, address indexed owner2);
​
function deployMultiSig(address owner1, address owner2) external returns (address) {
MultiSigWallet multiSig = new MultiSigWallet(owner1, owner2);
// Verify the deployment
require(address(multiSig).code.length > 0, "Deployment failed");
// Verify the initialization
require(multiSig.owner1() == owner1, "Invalid owner1");
require(multiSig.owner2() == owner2, "Invalid owner2");
emit MultiSigDeployed(address(multiSig), owner1, owner2);
return address(multiSig);
}
}

  1. Use a minimal proxy deployment pattern to reduce gas costs and failure risks:

contract MultiSigFactory {
address public immutable implementation;
constructor() {
implementation = address(new MultiSigWallet(address(0), address(0)));
}
function deployMultiSig(address owner1, address owner2) external returns (address) {
address proxy = Clones.clone(implementation);
MultiSigWallet(proxy).initialize(owner1, owner2);
require(MultiSigWallet(proxy).owner1() == owner1, "Invalid initialization");
return proxy;
}
}
Updates

Appeal created

n0kto Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_deployent_multisig_could_fail

No valid reason for the deployment to fail.

Support

FAQs

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