DatingDapp

First Flight #33
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: high
Invalid

Permanent Fund Lock Risk in DatingDapp MultiSig Contract

Description:

The MultiSigWallet contract lacks a mechanism to handle deadlock situations where owners cannot reach consensus, leading to permanently locked funds. This is especially critical in a dating context where:

  • Relationship breakdowns can lead to one party refusing to approve transactions

  • Both parties might stop communicating entirely

  • One party might become unresponsive or abandon the wallet

  • Spite or revenge motivations could lead to intentionally locked funds

  • One party has lesser funds in the wallet

Impact:

  • Users could permanently lose access to their deposited funds

  • No recourse if communication breaks down completely

  • Could be weaponized in toxic relationships to hold funds hostage

  • No way to recover individual contributions after relationship ends

Proof of Concept

function testPermanentlyLockedFunds() public {
// owner1 has 10 ether, owner2 has 1 ether
//mimicking likeregistry
vm.prank(address(this));
(bool success1,) = address(wallet).call{value: 11 ether}("");
require(success1, "Deposit failed");
// Relationship ends badly, owner2 refuses to approve any transactions
vm.startPrank(owner1);
wallet.submitTransaction(owner1, 10 ether); // Trying to withdraw their share
wallet.approveTransaction(0);
vm.stopPrank();
vm.startPrank(owner1);
vm.expectRevert("Not enough approvals");
wallet.executeTransaction(0);
vm.stopPrank();
// Owner2 never approves - funds are stuck forever
// No way to recover individual deposits
assertEq(address(wallet).balance, 11 ether);
//tries to get the funds after a year still fails because of lack of approval from the other party
vm.warp(365 days);
vm.startPrank(owner1);
vm.expectRevert("Not enough approvals");
wallet.executeTransaction(0);
vm.stopPrank();
assertEq(address(wallet).balance, 11 ether);
}

Recommended Mitigation Steps

Implement deposit tracking and time-based withdrawal safety:

contract MultiSigWallet {
//other codes
// Track individual deposits
mapping(address => uint256) public deposits;
// Time after which individual withdrawals are allowed
uint256 public constant LOCKUP_DURATION = 90 days; //this can be decided by the protocol
uint256 public createdTimestamp;
constructor(address _owner1, uint256 _owner1Amt, address _owner2, uint256 _owner2Amt) {
require(_owner1 != address(0) && _owner2 != address(0), "Invalid owner address");
require(_owner1 != _owner2, "Owners must be different");
owner1 = _owner1;
owner2 = _owner2;
deposits[_owner1] = _owner1Amt;
deposits[_owner2] = _owner2Amt;
createdTimestamp = block.timestamp;
}
/// @notice Emergency withdrawal after lockup period
function emergencyWithdraw() external {
require(msg.sender == owner1 || msg.sender == owner2, "Not an owner");
uint256 _amt = deposits[msg.sender];
require(block.timestamp >= createdTimestamp + LOCKUP_DURATION,
"Lockup period not ended");
require(_amt > 0, "No funds to withdraw");
deposits[msg.sender] = 0;
(bool success,) = payable(msg.sender).call{value: _amt}("");
require(success, "Withdrawal failed");
emit EmergencyWithdrawal(msg.sender, amount);
}
}

These changes ensure that funds can't be permanently locked in the contract, while still maintaining the multisig functionality during the normal relationship period.

Tools Used

Manual Review + Foundry Testing Framework

NB: This solution doesn't doesn't account for previoously executed transactions. You might need to modify the executeTransaction to reduces both parties' deposits equally after each calls.

Updates

Appeal created

n0kto Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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