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

No Mechanism to Change Owners in `MultiSigWallet` could cause DoS Attack

roccomania

Summary

The contract lacks a mechanism to change owners, which could render the wallet unusable if one of the owners becomes unresponsive or malicious. Thereby making funds stuck in the contract.

Vulnerability Details

The owner1 and owner2 addresses are set during contract deployment and cannot be updated afterward. If one of the owners loses access to their private key or refuses to approve transactions, the wallet could become permanently locked.

Impact

  • Loss of access to funds in the wallet.

  • Inability to execute transactions if one owner is uncooperative.

Tools Used

  • Manual code review.

Recommendations

Implement an Emergency Recovery Mechanism

Introduce a mechanism that allows the remaining active owner to propose a new owner if the other remains inactive for a specified period.

Solution: Add a "Timeout-Based Recovery" Function

  • Introduce a lastActivity timestamp for both owners.

  • Allow a waiting period (e.g., 30-90 days).

  • Add time stamp to Transaction struct

  • Make sure last transaction was inactive beyond the period and not executed yet.

  • If an owner remains inactive beyond this period, the other owner can propose a replacement.

mapping(address => uint256) public lastActivity;
uint256 public inactivityThreshold = 90 days;
​
modifier updateActivity() {
lastActivity[msg.sender] = block.timestamp;
_;
}
​
// Called when submitting, approving, or executing transactions
function updateOwnerActivity() public onlyOwners {
lastActivity[msg.sender] = block.timestamp;
}
​
function proposeNewOwner(address newOwner) external onlyOwners {
address inactiveOwner = (msg.sender == owner1) ? owner2 : owner1;
Transaction memory lastTxn = transactions[transactions.length - 1];
require(!lastTxn.executed, "Last txn already executed");
require(block.timestamp - lastActivity[inactiveOwner] > inactivityThreshold, "Owner still active");
​
if (msg.sender == owner1) {
owner2 = newOwner;
} else {
owner1 = newOwner;
}
}
Updates

Appeal created

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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