Lines of code
<https://github.com/multisig-labs/gogopool/blob/4bcef8b1d4e595c9ba41a091b2ebf1b45858f022/contracts/contract/RewardsPool.sol#L227&gt;

Vulnerability details

Impact

Transferring the allotAmount reward to MultisigManager leads to the loss of reward

Proof of Concept

If we refers to the original M-21 finding:

code-423n4/2022-12-gogopool-findings#143

Division by zero error can block RewardsPool#startRewardCycle if all multisig wallet are disabled.

Because of this line:

<https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/RewardsPool.sol#L229&gt;

	uint256 tokensPerMultisig = allotment / enabledCount;
	for (uint256 i = 0; i < enabledMultisigs.length; i++) {
		vault.withdrawToken(enabledMultisigs[i], ggp, tokensPerMultisig);
	}

The fix is that when there is no multisigWallet enabled, transfert he allotment to the multisigWallet

<https://github.com/multisig-labs/gogopool/blob/4bcef8b1d4e595c9ba41a091b2ebf1b45858f022/contracts/contract/RewardsPool.sol#L227&gt;

// If there are no enabled multisigs then the tokens will just sit under the multisig manager contract
if (enabledCount == 0) {
	vault.transferToken("MultisigManager", ggp, allotment);
	return;
}

However, if we look into the MultisigManager.sol implementation, there is no way to retrieve the allotment, then the reward that are meant to be distributed to the enabled multisig wallet is locked.

<https://github.com/multisig-labs/gogopool/blob/4bcef8b1d4e595c9ba41a091b2ebf1b45858f022/contracts/contract/MultisigManager.sol#L17&gt;

As we can see in the MultisigManager.sol, only the function related to enable / disable reward is implemented, I do not see in other part of the code, the implementation attempts to retrieve the balance from the MultisigManager.sol

When vault.transferToken is called

function transferToken(
	string memory networkContractName,
	ERC20 tokenAddress,
	uint256 amount
) external onlyRegisteredNetworkContract {
	// Valid Amount?
	if (amount == 0) {
		revert InvalidAmount();
	}
	// Make sure the network contract is valid (will revert if not)
	getContractAddress(networkContractName);
	// Get contract keys
	bytes32 contractKeyFrom = keccak256(abi.encodePacked(getContractName(msg.sender), tokenAddress));
	bytes32 contractKeyTo = keccak256(abi.encodePacked(networkContractName, tokenAddress));
	// emit token transfer event
	emit TokenTransfer(contractKeyFrom, contractKeyTo, address(tokenAddress), amount);
	// Verify there are enough funds
	if (tokenBalances[contractKeyFrom] < amount) {
		revert InsufficientContractBalance();
	}
	// Update Balances
	tokenBalances[contractKeyFrom] = tokenBalances[contractKeyFrom] - amount;
	tokenBalances[contractKeyTo] = tokenBalances[contractKeyTo] + amount;
}

token balance is removed from the contractKeyFrom and will never be added back

tokenBalances[contractKeyFrom] = tokenBalances[contractKeyFrom] - amount;

Tools Used

Manual Review

Recommended Mitigation Steps

If the enabledCount is 0, just return and do nothing, or make sure there is method to retrieve and add the reward amount hold in the MultisigManager.sol back to avoid the loss of reward.

---

The text was updated successfully, but these errors were encountered:

All reactions