DatingDapp
AI First Flight #6
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

`MultiSigWallet` has no mechanism to revoke approvals, preventing owners from changing their mind

lukamm

MultiSigWallet has no mechanism to revoke approvals, preventing owners from changing their mind

Description

The MultiSigWallet contract allows owners to approve transactions via MultiSigWallet::approveTransaction, but provides no way to revoke an approval once given. If an owner approves a transaction and later changes their mind (e.g., realizes the recipient address is wrong, the amount is incorrect, or circumstances have changed), they cannot undo their approval. This could lead to unintended transaction execution if the other owner approves without knowing the first owner no longer agrees.

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 {
if (txn.approvedByOwner2) revert AlreadyApproved();
txn.approvedByOwner2 = true;
}
​
emit TransactionApproved(_txId, msg.sender);
}
​
@> // No revokeApproval function exists

Risk

Likelihood:

  • An owner approves a transaction, then realizes the recipient or amount is incorrect

  • An owner approves, but circumstances change and they are not happy with the transaction

Impact:

  • Owners cannot change their mind after approving - approval is permanent until execution

  • Could lead to funds being sent to unintended recipients if the other owner approves without coordination

  • Reduces flexibility and safety of the multisig

Proof of Concept

  1. Alice and Bob match, receiving a multisig with their pooled funds

  2. Alice submits a transaction to send 1 ETH to address X

  3. Alice approves the transaction

  4. Alice realizes address X is compromised/incorrect

  5. Alice cannot revoke her approval

  6. Bob approves (unaware Alice changed her mind)

  7. Transaction can now be executed, sending funds to the wrong address

function testCannotRevokeApproval() public {
address owner1 = makeAddr("owner1");
address owner2 = makeAddr("owner2");
address recipient = makeAddr("recipient");
MultiSigWallet multiSig = new MultiSigWallet(owner1, owner2);
vm.deal(address(multiSig), 1 ether);
// Owner1 submits and approves
vm.startPrank(owner1);
multiSig.submitTransaction(recipient, 1 ether);
multiSig.approveTransaction(0);
vm.stopPrank();
// Owner1 realizes mistake - but cannot revoke!
// No revokeApproval function exists
// Owner2 approves without knowing Owner1 changed their mind
vm.prank(owner2);
multiSig.approveTransaction(0);
// Transaction can now be executed even though Owner1 no longer agrees
(,,bool approvedByOwner1, bool approvedByOwner2,) = multiSig.transactions(0);
assertTrue(approvedByOwner1 && approvedByOwner2);
}

Recommended Mitigation

Add a revokeApproval function to allow owners to withdraw their approval:

+ event TransactionApprovalRevoked(uint256 indexed txId, address indexed owner);
​
+ function revokeApproval(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) {
+ require(txn.approvedByOwner1, "Not approved by owner1");
+ txn.approvedByOwner1 = false;
+ } else {
+ require(txn.approvedByOwner2, "Not approved by owner2");
+ txn.approvedByOwner2 = false;
+ }
+
+ emit TransactionApprovalRevoked(_txId, msg.sender);
+ }
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 20 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!