Summary
Upon a match, wallet funds and fees are calculated. The latter can be claimed by the app owner. However, it's also possible to create and fund the MultiSigWallet
directly, avoiding paying 1 ETH for a like, as well as fees.
Vulnerability Details
Upon a match between two users, LikeRegistry::matchRewards
will get called.
function matchRewards(address from, address to) internal {
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;
multiSigWallet = new MultiSigWallet(from, to);
(bool success, ) = payable(address(multiSigWallet)).call{
value: rewards
}("");
require(success, "Transfer failed");
}
matchRewards
is calculating the funds for the shared wallet by adding all the previous like payments of the matched users, minus a 10% FIXED_FEE
that goes to the app owner via withdrawFees
.
function withdrawFees() external onlyOwner {
require(totalFees > 0, "No fees to withdraw");
uint256 totalFeesToWithdraw = totalFees;
console.log("totalFeesToWithdraw:", totalFeesToWithdraw);
totalFees = 0;
(bool success, ) = payable(owner()).call{value: totalFeesToWithdraw}(
""
);
require(success, "Transfer failed");
}
However, nothing prevents two users from creating and funding a MultiSigWallet
themselves,
contract MultiSigWallet {
modifier onlyOwners() {
if (msg.sender != owner1 && msg.sender != owner2) revert NotAnOwner();
_;
}
constructor(address _owner1, address _owner2) {
require(
_owner1 != address(0) && _owner2 != address(0),
"Invalid owner address"
);
require(_owner1 != _owner2, "Owners must be different");
owner1 = _owner1;
owner2 = _owner2;
}
receive() external payable {}
}
effectively bypassing any ETH-like payments in LikeRegistry::likeUser
, as well as app owner fees.
Proof of Concept
The following code demonstrates the ability of two app users to create a MultiSigWallet
, fund it, and perform transactions as they see fit. Place test_UsersCanCreateAndFundWallet
in testSoulboundProfileNFT.t.sol
:
function test_UsersCanCreateAndFundWallet() public {
address bob = makeAddr("bob");
address alice = makeAddr("alice");
vm.deal(bob, 10 ether);
vm.deal(alice, 10 ether);
vm.prank(bob);
soulboundNFT.mintProfile("Bob", 25, "ipfs://profileImage");
vm.prank(alice);
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
MultiSigWallet multiSigWallet = new MultiSigWallet(bob, alice);
vm.prank(bob);
(bool success, ) = payable(address(multiSigWallet)).call{
value: 1 ether
}("");
require(success, "Transaction failed");
vm.prank(alice);
(success, ) = payable(address(multiSigWallet)).call{value: 1 ether}("");
require(success, "Transaction failed");
assertEq(multiSigWallet.owner1(), bob);
assertEq(multiSigWallet.owner2(), alice);
assertEq(address(multiSigWallet).balance, 2 ether);
vm.startPrank(bob);
address restaurant = makeAddr("restaurant");
multiSigWallet.submitTransaction(restaurant, 1 ether);
(address transactionTo, uint256 transactionValue, , , ) = multiSigWallet
.transactions(0);
assertEq(transactionTo, restaurant);
assertEq(transactionValue, 1 ether);
multiSigWallet.approveTransaction(0);
vm.startPrank(alice);
multiSigWallet.approveTransaction(0);
multiSigWallet.executeTransaction(0);
assertEq(address(multiSigWallet).balance, 1 ether);
}
And run the test:
$ forge test --mt test_UsersCanCreateAndFundWallet
Ran 1 test for test/testSoulboundProfileNFT.t.sol:SoulboundProfileNFTTest
[PASS] test_UsersCanCreateAndFundWallet() (gas: 998243)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.02ms (1.50ms CPU time)
Ran 1 test suite in 147.60ms (8.02ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
This workaround defeats an essential feature of the app.
Tools used
Manual review, tests
Recommendations
Add proper checks in the constructor of MultiSigWallet
to make sure that wallet's owners are registered in DatingDapp (minted profile required) and that they like each other.
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
+ import "./LikeRegistry.sol";
contract MultiSigWallet {
// ...
modifier onlyOwners() {
if (msg.sender != owner1 && msg.sender != owner2) revert NotAnOwner();
_;
}
+
+ modifier onlyValidOwners(
+ address payable _likeRegistry,
+ address _owner1,
+ address _owner2
+ ) {
+ require(
+ _owner1 != address(0) && _owner2 != address(0),
+ "Invalid owner address"
+ );
+ require(_owner1 != _owner2, "Owners must be different");
+
+ LikeRegistry likeRegistry = LikeRegistry(_likeRegistry);
+ require(
+ likeRegistry.profileNFT().profileToToken(_owner1) != 0,
+ "_owner1 must have a profile NFT"
+ );
+ require(
+ likeRegistry.profileNFT().profileToToken(_owner2) != 0,
+ "_owner2 must have a profile NFT"
+ );
+ require(
+ likeRegistry.likes(_owner1, _owner2),
+ "_owner1 must like each _owner2_"
+ );
+ require(
+ likeRegistry.likes(_owner2, _owner1),
+ "_owner2 must like _owner1"
+ );
+ _;
+ }
+
constructor(
+ address payable _likeRegistry,
address _owner1,
address _owner2
+ ) onlyValidOwners(_likeRegistry, _owner1, _owner2) {
owner1 = _owner1;
owner2 = _owner2;
}
// ...
/// @notice Allows the contract to receive ETH
+ receive() external payable onlyOwners {}
}
Place test_UsersNeedProfileAndToMatchToCreateAWallet
in testSoulboundProfileNFT.t.sol
and run it with forge test --mt test_UsersNeedProfileAndToMatchToCreateAWallet
to verify proper construction of a MultiSigWallet
.
function test_UsersNeedProfileAndToMatchToCreateAWallet() public {
vm.deal(bob, 10 ether);
vm.deal(alice, 10 ether);
vm.expectRevert("_owner1 must have a profile NFT");
new MultiSigWallet(payable(address(likeRegistry)), bob, alice);
vm.prank(bob);
soulboundNFT.mintProfile("Bob", 25, "ipfs://profileImage");
vm.expectRevert("_owner2 must have a profile NFT");
new MultiSigWallet(payable(address(likeRegistry)), bob, alice);
vm.prank(alice);
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
vm.expectRevert("_owner1 must like _owner2");
new MultiSigWallet(payable(address(likeRegistry)), bob, alice);
vm.prank(bob);
likeRegistry.likeUser{value: 1 ether}(alice);
assertTrue(likeRegistry.likes(bob, alice));
vm.expectRevert("_owner2 must like _owner1");
new MultiSigWallet(payable(address(likeRegistry)), bob, alice);
vm.prank(alice);
likeRegistry.likeUser{value: 1 ether}(bob);
assertTrue(likeRegistry.likes(alice, bob));
new MultiSigWallet(payable(address(likeRegistry)), bob, alice);
}