175 lines
6.3 KiB
Markdown
175 lines
6.3 KiB
Markdown
# H-02. User Fund Sent to LikeRegistry::likeUser is Never Accounted and Make `LikeRegistry::matchRewards` Always Return Zero
|
|
|
|
## Summary
|
|
|
|
Due to miscalculation in the `LikeRegistry::matchRewards`, any deployed MultisigWallet will contain 0 fund, and also fees for the protocol is always 0 as well. User funds sent through `LikeRegistry::likeUser` is never being accounted to the user. It leads to the created multisigWallet has no fund in it, and user lose their fund.
|
|
|
|
## Vulnerability Detail
|
|
|
|
```solidity
|
|
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;
|
|
|
|
// Deploy a MultiSig contract for the matched users
|
|
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to);
|
|
|
|
// Send ETH to the deployed multisig wallet
|
|
(bool success,) = payable(address(multiSigWallet)).call{value: rewards}("");
|
|
require(success, "Transfer failed");
|
|
}
|
|
```
|
|
|
|
As we can see at the above snippet, `matchUserOne` and `matchUserTwo` are the main variables for this calculation. Its retrieved from the `userBalances` variable. But `userBalances` variable value actually never increased. As we can see in the below `LikeRegistry::likeUser` code:
|
|
|
|
```solidity
|
|
function likeUser(address liked) external payable {
|
|
[...]
|
|
|
|
likes[msg.sender][liked] = true;
|
|
emit Liked(msg.sender, liked);
|
|
|
|
userBalances[msg.sender] += msg.value;
|
|
|
|
// Check if mutual like
|
|
if (likes[liked][msg.sender]) {
|
|
matches[msg.sender].push(liked);
|
|
matches[liked].push(msg.sender);
|
|
emit Matched(msg.sender, liked);
|
|
address multiSigWallet = matchRewards(liked, msg.sender);
|
|
}
|
|
}
|
|
```
|
|
|
|
The `LikeRegistry::likeUser` require user to send fund greater than or equal to 1 ether, but these 1 ether never being accounted to the sender address.
|
|
|
|
## POC
|
|
|
|
We modify the `src/LikeRegistry.sol` file a bit to return the multisigWallet address from `likeUser` and `matchRewards`.
|
|
|
|
`LikeRegistry::likeUser`
|
|
|
|
```diff
|
|
- function likeUser(address liked) external payable {
|
|
+ function likeUser(address liked) external payable returns (address) {
|
|
require(msg.value >= 1 ether, "Must send at least 1 ETH");
|
|
require(!likes[msg.sender][liked], "Already liked");
|
|
require(msg.sender != liked, "Cannot like yourself");
|
|
@@ -38,16 +38,23 @@ contract LikeRegistry is Ownable {
|
|
likes[msg.sender][liked] = true;
|
|
emit Liked(msg.sender, liked);
|
|
|
|
// Check if mutual like
|
|
if (likes[liked][msg.sender]) {
|
|
matches[msg.sender].push(liked);
|
|
matches[liked].push(msg.sender);
|
|
emit Matched(msg.sender, liked);
|
|
- matchRewards(liked, msg.sender);
|
|
+ address multiSigWallet = matchRewards(liked, msg.sender);
|
|
+ return multiSigWallet;
|
|
}
|
|
+
|
|
+ return address(0);
|
|
}
|
|
```
|
|
|
|
`LikeRegistry::matchRewards`
|
|
|
|
```solidity
|
|
- function matchRewards(address from, address to) internal {
|
|
+ function matchRewards(address from, address to) internal returns (address) {
|
|
uint256 matchUserOne = userBalances[from];
|
|
uint256 matchUserTwo = userBalances[to];
|
|
userBalances[from] = 0;
|
|
@@ -64,6 +71,8 @@ contract LikeRegistry is Ownable {
|
|
// Send ETH to the deployed multisig wallet
|
|
(bool success,) = payable(address(multiSigWallet)).call{value: rewards}("");
|
|
require(success, "Transfer failed");
|
|
+
|
|
+ return address(multiSigWallet);
|
|
}
|
|
```
|
|
|
|
`test/testLikeRegistry.t.sol`
|
|
|
|
```solidity
|
|
function testMatchMiscalculation() public {
|
|
uint256 likeAmount = 1 ether;
|
|
uint256 FIXEDFEE = 10;
|
|
uint256 fees = ((likeAmount + likeAmount) * FIXEDFEE) / 100;
|
|
uint multiSigFund = likeAmount + likeAmount - fees;
|
|
|
|
// user mint profile
|
|
vm.prank(user);
|
|
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
|
|
uint256 userTokenId = soulboundNFT.profileToToken(user);
|
|
assertEq(userTokenId, 1, "Token ID should be 1");
|
|
|
|
// user2 mint profile
|
|
vm.prank(user2);
|
|
soulboundNFT.mintProfile("Bob", 27, "ipfs://profileImage");
|
|
uint256 user2TokenId = soulboundNFT.profileToToken(user2);
|
|
assertEq(user2TokenId, 2, "Token ID should be 2");
|
|
|
|
// likeRegistry has 0 ether
|
|
assertEq(address(likeRegistry).balance, 0);
|
|
|
|
// user likes user2 with 1 eth
|
|
vm.prank(user);
|
|
likeRegistry.likeUser{value: likeAmount}(user2);
|
|
|
|
// user2 likes uer with 1 eth and match
|
|
vm.prank(user2);
|
|
address multiSigAddress = likeRegistry.likeUser{value: likeAmount}(user);
|
|
assert(multiSigAddress != address(0));
|
|
|
|
// get user matches
|
|
vm.prank(user);
|
|
address[] memory userMatches = likeRegistry.getMatches();
|
|
assertEq(userMatches.length, 1);
|
|
assertEq(userMatches[0], address(user2));
|
|
|
|
assertEq(address(multiSigAddress).balance, multiSigFund); // giving a like for each user 1 ether should give the wallet 1.8 ether
|
|
assertEq(address(likeRegistry).balance, fees); // likeRegistry should have balance equal to `fees`, and send the rest of fund to the MultisigWallet
|
|
}
|
|
```
|
|
|
|
## Impact
|
|
|
|
- The user sending fund to `LikeRegistry::likeUser` has its fund never accounted to, which make a financial losses to the user
|
|
- The new created MultisigWallet has no fund in it
|
|
- Protocol unable to claim the protocol fee with `LikeRegistry::withdrawFees` despite the protocol balance increases
|
|
|
|
## Recommendations
|
|
|
|
Account user fund when user called `LikeRegistry::likeUser` to a storage variable.
|
|
|
|
```diff
|
|
function likeUser(address liked) external payable {
|
|
require(msg.value >= 1 ether, "Must send at least 1 ETH");
|
|
require(!likes[msg.sender][liked], "Already liked");
|
|
require(msg.sender != liked, "Cannot like yourself");
|
|
@@ -38,16 +38,23 @@ contract LikeRegistry is Ownable {
|
|
likes[msg.sender][liked] = true;
|
|
emit Liked(msg.sender, liked);
|
|
|
|
+ userBalances[msg.sender] += msg.value;
|
|
+
|
|
// Check if mutual like
|
|
if (likes[liked][msg.sender]) {
|
|
matches[msg.sender].push(liked);
|
|
matches[liked].push(msg.sender);
|
|
emit Matched(msg.sender, liked);
|
|
matchRewards(liked, msg.sender);
|
|
}
|
|
}
|
|
```
|
|
|