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

M-03. Even if not an owner, a MultiSig Wallet event can still approve a transaction

0xharry

Summary

MultiSig::approveTransaction requires only owners to approve the transaction, but the conditional judgment is incorrect. As a result, except for owners, others can also approve the transaction.

Vulnerability Details

Others, except for owner1, can approve the transaction using owner2 identity.

function approveTransaction(uint256 _txId) external onlyOwners {
require(_txId < transactions.length, "Invalid transaction ID");
Transaction storage txn = transactions[_txId];
require(!txn.executed, "Transaction already executed");
​
if (msg.sender == owner1) {
if (txn.approvedByOwner1) revert AlreadyApproved();
txn.approvedByOwner1 = true;
} else { // everyone can approve transaction
if (txn.approvedByOwner2) revert AlreadyApproved();
txn.approvedByOwner2 = true;
}
​
emit TransactionApproved(_txId, msg.sender);
}

Impact

The MultiSig wallet has only two owners, with others acting as a single owner, setting a 50% security threshold. This means that only one owner can execute transactions and use the wallet's funds.

Tools Used

Manual review

Foundry for POC

Recommendations

There needs to be a check to verify if the address is owner2, not just that all others are owner2.

function approveTransaction(uint256 _txId) external onlyOwners {
require(_txId < transactions.length, "Invalid transaction ID");
Transaction storage txn = transactions[_txId];
require(!txn.executed, "Transaction already executed");
​
if (msg.sender == owner1) {
if (txn.approvedByOwner1) revert AlreadyApproved();
txn.approvedByOwner1 = true;
- } else {
+ } else if(msg.sender == owner2) {
if (txn.approvedByOwner2) revert AlreadyApproved();
txn.approvedByOwner2 = true;
}
​
emit TransactionApproved(_txId, msg.sender);
}
Updates

Appeal created

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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