58 lines
2.4 KiB
Markdown
58 lines
2.4 KiB
Markdown
# Fund Locked in MultiSig when Match with a Blocked Profile
|
|
|
|
## This is an invalid high finding because of design choice. Written by myself.
|
|
|
|
## Summary
|
|
|
|
A profile can be blocked by the owner through `SoulboundProfileNFT::blockProfile`. But the fact that the profile is being blocked, doesn't change anything on the MultiSIgWallet. Someone who match up with a blocked profile now in the risk of their fund will never be able to withdraw. There is no other way to withdraw fund from MultiSigWallet other than both of the owner approve the transaction.
|
|
|
|
## Vulnerability Details
|
|
|
|
When user being blocked by owner through `SoulBoundProfileNFT::blockProfile`, it doesn't change any state of the `MultiSigWallet`. Transaction execution from `MultiSigWallet::executeTransaction` require both of party to sign through `MultiSigWallet::approveTransaction`.
|
|
|
|
```solidity
|
|
function executeTransaction(uint256 _txId) external onlyOwners {
|
|
require(_txId < transactions.length, "Invalid transaction ID");
|
|
Transaction storage txn = transactions[_txId];
|
|
require(!txn.executed, "Transaction already executed");
|
|
require(txn.approvedByOwner1 && txn.approvedByOwner2, "Not enough approvals");
|
|
|
|
[...]
|
|
}
|
|
```
|
|
|
|
## POC
|
|
|
|
```solidity
|
|
function testUserInRelationshipWithBlockedProfileUnableToWithdraw() public {
|
|
uint256 initialMultiSigWalletBalance = address(multiSigWallet).balance;
|
|
uint256 initialUserBalance = address(user).balance;
|
|
uint256 withdrawBalance = initialMultiSigWalletBalance / 2;
|
|
|
|
vm.startPrank(user);
|
|
// submit tx
|
|
multiSigWallet.submitTransaction(address(user), withdrawBalance);
|
|
|
|
// approve tx
|
|
multiSigWallet.approveTransaction(0);
|
|
|
|
// execute
|
|
vm.expectRevert();
|
|
multiSigWallet.executeTransaction(0);
|
|
vm.stopPrank();
|
|
|
|
(address to, uint256 value, bool approvedByOwner1, bool approvedByOwner2, bool executed) = multiSigWallet.transactions(0);
|
|
assertEq(executed, false); // unable to execute because need both of approval, despite the other user is already blocked
|
|
assertEq(address(user).balance, initialUserBalance + withdrawBalance); // user should be able to withdraw their fund from multiSigWallet from a blocked profile
|
|
}
|
|
```
|
|
|
|
## Impact
|
|
|
|
- Lost of funds because funds are locked in the MultiSigWallet and require approval from the blocked profile which might rejects all the transaction requests that benefit other user
|
|
|
|
## Recommendations
|
|
|
|
Add a function to let user withdraw funds to their wallet if their match up is blocked.
|
|
|