Submission Details
Severity:
Any account can send ETH to the `multiSignatureWallet` created for users.
Summary
The `MultiSigWallet` contract should be funded by the `LikeRegistry` contract depending on the pooled balances of each matched user. However, the `MultiSigWallet` can be funded by anyone beating its original functionality.
Vulnerability Details
Proof of Code:
<details>
<summary>Code</summary>
Add the following code to the `testSoulboundProfileNFT.t.sol` file.
```javascript
// Add the following code to the `LikeRegistry.sol` file for the test to run.
function likeUser(address liked) external payable returns (MultiSigWallet multiSigWallet) {
//user can send any amount, why?
require(msg.value >= 1 ether, "Must send at least 1 ETH");
//@ochuko -> potential bug
require(!likes[msg.sender][liked], "Already liked");
require(msg.sender != liked, "Cannot like yourself");
require(profileNFT.profileToToken(msg.sender) != 0, "Must have a profile NFT");
require(profileNFT.profileToToken(liked) != 0, "Liked user must have a profile NFT");
userBalances[liked] += msg.value;
likes[msg.sender][liked] = true;
emit Liked(msg.sender, liked);
if (likes[liked][msg.sender]) {
matches[msg.sender].push(liked);
matches[liked].push(msg.sender);
emit Matched(msg.sender, liked);
multiSigWallet = matchRewards(liked, msg.sender);
}
return multiSigWallet;
}
function matchRewards(address from, address to) internal returns (MultiSigWallet) {
uint256 matchUserOne = userBalances[from];
uint256 matchUserTwo = userBalances[to];
userBalances[from] = 0;
userBalances[to] = 0;
uint256 totalRewards = matchUserOne + matchUserTwo;
uint256 matchingFees = (totalRewards * FIXEDFEE) / 100;
uint256 rewards = totalRewards - matchingFees;
totalFees += matchingFees;
// Deploy a MultiSig contract for the matched users
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to, address(this));
// Send ETH to the deployed multisig wallet
(bool success,) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
return multiSigWallet;
}
function testAnyOneCanSendFundsToMultiSigWallet() public {
LikeRegistry userRegistry = new LikeRegistry(address(soulboundNFT));
vm.prank(user); // Simulates user calling the function
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
vm.prank(user2); // Simulates user2 calling the function
soulboundNFT.mintProfile("Ochuko", 24, "ipfs://profileImage");
vm.deal(user, 4 * TRANSFER_AMOUNT);
vm.deal(user2, 4 * TRANSFER_AMOUNT);
vm.deal(user3, 4 * TRANSFER_AMOUNT);
vm.prank(user);
userRegistry.likeUser{value: TRANSFER_AMOUNT}(user2);
vm.prank(user2);
MultiSigWallet multiSigWallet2 = userRegistry.likeUser{value: TRANSFER_AMOUNT}(user);
vm.prank(user);
(bool success,) = payable(address(multiSigWallet2)).call{value: TRANSFER_AMOUNT}("");
require(success, "Transfer failed");
vm.prank(user2);
(bool success2,) = payable(address(multiSigWallet2)).call{value: TRANSFER_AMOUNT}("");
require(success2, "Transfer failed");
vm.prank(user3);
(bool success3,) = payable(address(multiSigWallet2)).call{value: TRANSFER_AMOUNT}("");
require(success3, "Transfer failed");
uint256 amountLeftAfterSendingToMultiSigAccount = (2 * TRANSFER_AMOUNT * 10) / 100;
uint256 amountSent = (2 * TRANSFER_AMOUNT * 90) / 100;
uint256 multiSigWallet2AfterReceivingETH = amountSent + (3 * TRANSFER_AMOUNT);
assertEq(amountLeftAfterSendingToMultiSigAccount, address(userRegistry).balance);
assertEq(multiSigWallet2AfterReceivingETH, address(multiSigWallet2).balance);
}
```
</details>
Impact
This will cause people to lose money as anybody can mistakenly transfer money into the contract thinking it is a normal wallet.
Recommendations
To fix this, we can add the deployer address to each multiSignature wallet and check that only the deployer is able to send in ETH. Add the following code to the `MultiSig.sol` file.
```diff
//add a deployer variable to the MultiSigWallet contract
+ address public deployer;
+ constructor(address _owner1, address _owner2, address _deployer) {
- constructor(address _owner1, address _owner2) {
+ require(_deployer != address(0), "Invalid deployer address");
+ require(_deployer != _owner1 && _deployer != owner2, "Deployer and owners must be different");
require(_owner1 != address(0) && _owner2 != address(0), "Invalid owner address");
require(_owner1 != _owner2, "Owners must be different");
owner1 = _owner1;
owner2 = _owner2;
+ deployer = _deployer;
}
+ modifier onlyDeployer() {
+ if (msg.sender != deployer) revert NotDeployer();
+ _;
+ }
+ receive() external payable {}
+ receive() external payable onlyDeployer {}
```
Updates
Appeal created
n0kto 7 months ago
Submission Judgement Published Reason: Non-acceptable severity
Assigned finding tags:
Users mistake, only impacting themselves.
Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.
Rewards breakdown
nSLOC
197This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.