WinWin Protocol - WinWin

The PrizePool contract implements the drawRandomNumber() function which is used to set a draw as completed:

Moreover, the function allows passing an array of _slots as parameter. However:

1. This function is an external function that can be called by anyone.

2. claimWinning() function can be called right away after the drawRandomNumber() call.

3. RandomNumbers used to determine the winner will be known at this time.

Consequently, a malicious user can tamper with the final winning results by playing with the _slots parameter passed to the drawRandomNumber() call as this parameter can be used to:

1. Fill empty ticket slots.

2. Change the lastDrawEligibleCount() used.

For example, let's imagine the following scenario:

1. stakeIntoWinStakingPool(user1, 100e18);

2. stakeIntoWinStakingPool(user2, 200e18);

3. stakeIntoWinStakingPool(user3, 100e18);

4. unstakeFromWinStakingPool(user2, 200e18);

5. stakeIntoWinStakingPool(user2, 200e18);

6. stakeIntoWinStakingPool(user3, 200e18);

Calling drawRandomNumber() with an empty _slots array:

Calling drawRandomNumber() with a non-empty _slots array. _slots array = [0]:

Notice how the lastDrawEligibleCount is different, which can be abused to tamper with the final winning results.

It is recommended to add some type of access control to the drawRandomNumber() function. Moreover, consider removing the _slots array passed as parameter.

Remediation Plan

SOLVED: The WinWin team solved the issue by updating RandomNumber contract to store hashes/random number against last request Id.

The HEXStrategy contract implements a FIFO withdrawal queue. Every time a user calls the unstake() function in the HEXStakingPool the withdraw() function is called in the HEXStrategy contract:

This function, always, performs a call to the _clearWithdrawQueue() function in order to try to empty the withdrawal queue. This is either achieved through the first if or the else code block. We can see below the _clearWithdrawQueue() function:

However, if in the new period the currentTotalStakeAmount is not bigger than the current amount withdrawn in the first position of the withdrawal queue, the withdrawal will not be executed. The else code block will be entered and the rest of the withdrawals in the queue will not be processed. Consequently, let's imagine the following scenario:

1. Multiple users stake.

2. One user stakes a very high amount, ie. 2.000.000.000 HEX tokens.

3. 30 days later (claim is called) and many users start unstaking.

4. First user calling unstake is the one that staked the 2.000.000.000 HEX tokens. After that the rest of the users.

5. All the withdraw requests will be blocked until currentTotalStakeAmount is higher than the 2.000.000.000 HEX tokens.

Moreover, in the next claim cycle (30 days later), all the stakes/unstakes will be blocked as they will all try to trigger the claimAndDistributeRewards() function. This function internally calls the stakeHEX() function:

An underflow will occur in the _currentTotalStakeAmount - withdrawQueueTotalAmount line as the withdrawQueueTotalAmount will be higher than the _currentTotalStakeAmount:

Consider refactoring the withdrawal queue logic in the HEXStrategy contract to prevent the underflow and the block of all the withdrawals in the queue.

Remediation Plan

SOLVED: The WinWin team solved the issue by adding a specific check against this issue and updated the logic to calculate yield if there is no transaction for 2 cycles.

The contract BuyAndBurn implements the function swap():

This function swaps the different reward tokens received every 30 days by the BuyAndBurn contract to Wins tokens to then send them to a burn/dead address.

These reward tokens are sent to the contract through the claimAndDistributeRewards() function implemented by the different strategies. This function is a public function that can be called by anyone. Consequently, the following exploit is possible:

• The path for the HEX tokens received as rewards are set by an admin to HEX => WPLS => WINS.

• A malicious user buys WINS from the WPLS/WINS pool. Pool reserves of WINS decrement, Pool reserves of WPLS increment.

• Within the same transaction BuyAndBurn.swap() is called. Pool reserves of WINS decrement further, Pool reserves of WPLS increment further.

• The malicious user sells all the WINS to the WPLS/WINS pool at an inflated price caused by the previous swap call.

It is recommended to add some type of access control to the BuyAndBurn.swap() function so it can only be called by an admin.

Remediation Plan

RISK ACCEPTED: The WinWin team accepted the risk of this finding.

The USDLStrategy contract implements the function _swapPLSforUSDL():

This function is used to swap the PLS tokens received as yield for USDL. The amountOutMin parameter used in the swapExactETHForTokens() is calculated on-chain. Consequently this can still be subject to manipulation as if the transaction is front-run the IUniswapV2Router(routerAddress).getAmountsOut(_rewardPLS, path); call will be performed on possibly manipulated pool reserves.

With this implementation, any deposit/withdrawal/claim call (as these 3 functions internally calls the _swapPLSforUSDL() function) could be sandwiched in detriment of the stakers that would receive lower rewards.

This issue can only have a significant impact if no deposits/withdrawals/claims are executed for a long time and if the pool reserves are relatively low to the rewards accrued. This scenario should be very unlikely to ever occur, however, the WinWin team should be aware that the current implementation of the USDLStrategy to avoid slippage in the swaps does not work as intended and every swap call can be sandwiched (even if the profit will be insignificant for the attackers).

Remediation Plan

RISK ACCEPTED: The WinWin team accepted the risk of this finding.

In the claimWinning() function the winner is selected by dividing randomNumber[i] by ticketInfo.lastDrawEligibleCount:

In case of an exact division, winningIndex will be zero. In this situation, no winner is determined and the rewards will be distributed later on through the claimPrizes() function.

Consider implementing a mechanism to re-calculate winningIndex when it is zero enforcing that there will be always a winner selected for a certain tier.

Remediation Plan

RISK ACCEPTED: The WinWin team accepted the risk of this finding and stated that no fix is required as there are already 5 random numbers for each tier.

The different staking contracts implement the function stake():

Every time that this function is called and the unlockTime was not reached a new element is added to the Balance[] array that belong to the following mapping declared in the contract storage: mapping(address => Balance[]) private _balances;

However, the claim() function, used to claim the rewards for the user, iterates over all the elements of that array:

If this array is large enough, the block gas limit may be reached. This can lead to failed transactions and a denial of service for users attempting to claim their rewards. The block gas limit is a hard limit on the amount of computation a single transaction can use. When the number of elements in the Balance[] array grows excessively, the gas required to process the entire array in the claim() function may exceed this limit, preventing the function from completing successfully.

It is recommended to limit the number of stakes that a user can perform.

Remediation Plan

RISK ACCEPTED: The WinWin team accepted the risk of this finding and stated that a disclaimer will be added in the frontend.

The current ownership transfer process for all the contracts inheriting from Ownable or OwnableUpgradeable involves the current owner calling the transferOwnership() function:

function transferOwnership(address newOwner) public virtual onlyOwner {
    require(newOwner != address(0), "Ownable: new owner is the zero address");
    _setOwner(newOwner);
}

If the nominated EOA account is not a valid account, it is entirely possible that the owner may accidentally transfer ownership to an uncontrolled account, losing the access to all functions with the onlyOwner modifier.

It is recommended to implement a two-step process where the owner nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of the ownership to fully succeed. This ensures the nominated EOA account is a valid and active account. This can be easily achieved by using OpenZeppelin's Ownable2Step contract instead of Ownable:

Remediation Plan

SOLVED: The WinWin team solved the issue by implementing the recommended solution.

If a loop is iterated many times, the amount of gas required to execute the function increases significantly. In Solidity, excessive looping can cause a function to use more than the maximum allowed gas, which causes the function to fail.

RandomNumber.sol

• Line 32: for (uint256 i = 0; i < _authorized.length; i++) {

LOANStrategy.sol

• Line 108: for (uint8 i = 0; i < 5; i++) {

• Line 162: for (uint256 i = 0; i < 5; i++) {

• Line 185: for (uint256 i = 0; i < 5; i++) {

MINTRAStrategy.sol

• Line 106: for (uint8 i = 0; i < 5; i++) {

• Line 162: for (uint256 i = 0; i < 5; i++) {

• Line 185: for (uint256 i = 0; i < 5; i++) {

USDLStrategy.sol

• Line 116: for (uint8 i = 0; i < 5; i++) {

• Line 238: for (uint256 i = 0; i < 5; i++) {

• Line 261: for (uint256 i = 0; i < 5; i++) {

HEXStrategy.sol

• Line 139: for (uint8 i = 0; i < 5; i++) {

• Line 238: for (uint256 j = 0; j < totalStakeCount; j++) {

• Line 252: for (uint256 j = 0; j < totalStakeCount; j++) {

• Line 257: for (uint256 k = 0; k < newTotalStakeCount; k++) {

• Line 352: for (uint256 i = 0; i < 5; i++) {

• Line 375: for (uint256 i = 0; i < 5; i++) {

• Line 678: for (index = startingIndex; index < (startingIndex + length); index++) {

WinStakingPool.sol

• Line 194: for (uint256 i = 1; i < _rewardTokens.length; i++) {

• Line 401: for (uint256 i = 1; i < _rewardTokens.length; i++) {

• Line 456: for (uint256 i = index; i < bal.length; i++) {

• Line 468: for (uint256 j = 1; j < _rewardTokens.length; j++) {

• Line 556: for (uint256 i = 1; i < _rewardTokens.length; i++) {

• Line 604: for (uint256 j = 1; j < _rewardTokens.length; j++) {

• Line 606: for (uint256 i = index; i < bal.length; i++) {

• Line 820: for (uint256 i = index; i < bal.length; i++) {

• Line 868: for (uint256 j = 0; j < _rewardsData.length; j++) {

• Line 869: for (uint256 i = index; i < bal.length; i++) {

• Line 909: for (uint256 j = 0; j < _rewardsData.length; j++) {

• Line 910: for (uint256 i = index; i < bal.length; i++) {

• Line 955: for (uint256 j = 1; j < _rewardsData.length; j++) {

• Line 995: for (uint256 i = index; i < bal.length; i++) {

• Line 1013: for (uint256 j = 1; j < _rewardTokens.length; j++) {

• Line 1089: for (uint256 i = index; i < bal.length; i++) {

• Line 1292: for (uint256 i = 0; i < numberOfDays; i++) {

HEXStakingPool.sol

• Line 142: for (uint256 i = 1; i < _rewardTokens.length; i++) {

• Line 360: for (uint256 i = 1; i < _rewardTokens.length; i++) {

• Line 418: for (uint256 i = index; i < bal.length; i++) {

• Line 430: for (uint256 j = 1; j < _rewardTokens.length; j++) {

• Line 522: for (uint256 i = 1; i < _rewardTokens.length; i++) {

• Line 569: for (uint256 j = 1; j < _rewardTokens.length; j++) {

• Line 572: for (uint256 i = index; i < bal.length; i++) {

• Line 762: for (uint256 i = index; i < bal.length; i++) {

• Line 809: for (uint256 j = 0; j < _rewardsData.length; j++) {

• Line 811: for (uint256 i = index; i < bal.length; i++) {

• Line 850: for (uint256 j = 0; j < _rewardsData.length; j++) {

• Line 852: for (uint256 i = index; i < bal.length; i++) {

• Line 897: for (uint256 j = 1; j < _rewardsData.length; j++) {

• Line 937: for (uint256 i = index; i < bal.length; i++) {

• Line 955: for (uint256 j = 1; j < _rewardTokens.length; j++) {

• Line 1015: for (uint256 i = index; i < bal.length; i++) {

StakingPool.sol

• Line 106: for (uint256 i = 0; i < _rewardTokens.length; i++) {

• Line 289: for (uint256 i = 0; i < _rewardTokens.length; i++) {

• Line 340: for (uint256 i = index; i < bal.length; i++) {

• Line 354: for (uint256 j = 0; j < _rewardTokens.length; j++) {

• Line 407: for (uint256 i = 0; i < _rewardTokens.length; i++) {

• Line 456: for (uint256 i = 0; i < _rewardTokens.length; i++) {

• Line 458: for (uint256 j = index; j < bal.length; j++) {

• Line 531: for (uint256 i = index; i < bal.length; i++) {

• Line 550: for (uint256 j = 0; j < _rewardTokens.length; j++) {

• Line 691: for (uint256 i = 0; i < _rewardsData.length; i++) {

• Line 693: for (uint256 j = index; j < bal.length; j++) {

• Line 716: for (uint256 i = 0; i < _unlockReward.length; i++) {

• Line 718: for (uint256 j = index; j < bal.length; j++) {

• Line 744: for (uint256 j = 0; j < _rewardsData.length; j++) {

• Line 764: for (uint256 j = index; j < bal.length; j++) {

BuyAndBurn.sol

• Line 118: for (uint i = 0; i < tokenAddresses.length; i++) {

PrizePool.sol

• Line 208: for (uint256 i = 0; i < pointer; i++) {

• Line 241: for (int256 i = int256(length - 1); i >= 0; i--) {

• Line 334: for (uint256 i = 0; i < pointer; i++) {

• Line 353: for (uint256 j = 0; j < returnedPointer; j++) {

• Line 364: for (uint256 i = 0; i < deletedIndexPointer; i++) {

• Line 383: for (uint256 i = 0; i < _draw.length; i++) {

• Line 398: for (uint256 j = 0; j < 5; j++) {

• Line 403: for (uint256 k = 0; k < rewardInfo.rewardTokens.length; k++)

• Line 439: for (uint256 k = 0; k < rewardInfo.rewardTokens.length; k++) {

• Line 463: for (uint256 k = 0; k < rewardInfo.rewardTokens.length; k++) {

• Line 513: for (uint256 k = 0; k < _alreadyWinners.length; k++) {

• Line 544: for (uint j = 0; j < _alreadyWinners.length; j++) {

• Line 580: for (i = 0; i < rewardInfo.rewardTokens.length; i++) {

• Line 618: for (uint256 i = 0; i < _emptyArray.length; i++) {

• Line 691: for (uint i = startingIndex; i < endingIndex; i++) {

• Line 787: for (uint256 i = 0; i < _draw.length; i++) {

• Line 800: for (uint256 j = 0; j < 5; j++) {

• Line 805: for (uint256 k = 0; k < rewardInfo.rewardTokens.length;k++) {

• Line 833: for (uint256 k = 0; k < rewardInfo.rewardTokens.length; k++) {

• Line 975: for (uint256 i = 0; i < _slots.length; i++) {

• Line 1039: for (uint i = 0; i < _slots.length; i++) {

• Line 1057: for (uint256 j = 0; j < returnedPointer; j++) {

• Line 1066: for (uint256 i = 0; i < pointer; i++) {

To reduce gas consumption, it is recommended to find ways to optimize the loop or potentially break the loop into smaller batches. The following pattern can also be used:

uint256 cachedLen = array.length;
for(uint i; i < cachedLen;){
  unchecked {
    ++i;
  }
}

Remediation Plan

SOLVED: The WinWin Team solved the issue by implementing the suggested recommendation.

The staking pool contracts implement the function getBalanceAt():

This function downcasts the uint64 parameter to an uint32 as is the type required by the TwabLib.getBalanceAt() function. Consequently, in order to avoid the downcast and any possible overflow during the type casting, it is recommended to declare the _target parameter directly as an uint32.

Consider declaring the _target parameter directly as an uint32 in the getBalanceAt() function.

Remediation Plan

SOLVED: The WinWin team solved the issue by implementing the recommended solution.

In the current implementation of the protocol the winners are determined based of random numbers generated off-chain which then are added to the RandomNumber contract through the setRandomNumber() function, only callable by an admin:

Using off-chain random number generation for determining the winners introduces several significant issues:

1. Centralization Risk:

   • The reliance on an off-chain source for randomness and the need for an admin to set the random number means that the entire system is dependent on a central authority. This central point of control can be exploited or corrupted.

2. Lack of Transparency:

   • Participants in the raffle have no way to verify the fairness of the random number. The process is opaque, and users must trust the admin to act honestly, which undermines the trustless nature of blockchain technology.

3. Potential for Manipulation:

   • The admin, or anyone with control over the off-chain random number generator, can manipulate the outcome of the raffle by choosing favorable random numbers. This opens the door to fraud and reduces the integrity of the protocol.

4. Single Point of Failure:

   • If the off-chain random number generator or the admin’s ability to set the random number is compromised, the entire raffle system can be disrupted.

Alternatives for Secure and Trustless Random Number Generation

1. Chainlink VRF (Verifiable Random Function):

   • Description: Chainlink VRF provides a provably fair and verifiable source of randomness. It is a decentralized oracle service that generates random numbers that can be verified by anyone.

   • Implementation:

     import "@chainlink/contracts/src/v0.8/VRFConsumerBase.sol";
     
     contract Raffle is VRFConsumerBase {
         bytes32 internal keyHash;
         uint256 internal fee;
         uint256 public randomResult;
     
         constructor() 
             VRFConsumerBase(
                 0x514910771AF9Ca656af840dff83E8264EcF986CA, // VRF Coordinator
                 0x514910771AF9Ca656af840dff83E8264EcF986CA  // LINK Token
             ) {
             keyHash = 0xAA77729D3466CA35AE8D28FA0D0B6AEE3B7D2B13;
             fee = 0.1 * 10 ** 18; // 0.1 LINK
         }
     
         function getRandomNumber() public returns (bytes32 requestId) {
             require(LINK.balanceOf(address(this)) >= fee, "Not enough LINK");
             return requestRandomness(keyHash, fee);
         }
     
         function fulfillRandomness(bytes32 requestId, uint256 randomness) internal override {
             randomResult = randomness;
         }
     }

   • Pros: Decentralized, secure, and verifiable.

   • Cons: Requires LINK tokens and depends on the Chainlink network.

2. Ethereum Beacon Chain Randomness (RANDAO):

   • Description: The Ethereum 2.0 Beacon Chain includes a randomness beacon that can be used for generating random numbers. It leverages the RANDAO protocol for secure and decentralized randomness.

   • Implementation: This method is more suitable for contracts deployed on Ethereum 2.0 and would require interfacing with the Beacon Chain’s randomness features.

3. Commit-Reveal Scheme:

   • Description: This scheme involves participants committing to a random number without revealing it, then revealing the number in a later transaction. The final random number is derived from these committed numbers.

   • Implementation:

     contract Raffle {
         struct Commitment {
             bytes32 hash;
             bool revealed;
             uint256 number;
         }
         mapping(address => Commitment) public commitments;
     
         function commit(bytes32 hash) public {
             commitments[msg.sender] = Commitment({hash: hash, revealed: false, number: 0});
         }
     
         function reveal(uint256 number) public {
             Commitment storage commitment = commitments[msg.sender];
             require(commitment.hash == keccak256(abi.encodePacked(number)), "Invalid reveal");
             require(!commitment.revealed, "Already revealed");
             commitment.revealed = true;
             commitment.number = number;
         }
     
         function determineWinner() public view returns (address winner) {
             // Logic to determine the winner based on revealed numbers
         }
     }

   • Pros: Decentralized and trustless.

   • Cons: More complex and requires participant involvement in both commit and reveal phases.

4. Using Blockhash:

   • Description: Use the hash of a future block as a source of randomness.

   • Example implementation:

     // This code has not been professionally audited, therefore I cannot make any promises about
     // safety or correctness. Use at own risk.
     contract Randomness {
     
         bytes32 sealedSeed;
         bool seedSet = false;
         bool betsClosed = false;
         uint storedBlockNumber;
         address trustedParty = 0xdCad3a6d3569DF655070DEd06cb7A1b2Ccd1D3AF;
     
         function setSealedSeed(bytes32 _sealedSeed) public {
             require(!seedSet);
             require (msg.sender == trustedParty);
             betsClosed = true;
             sealedSeed = _sealedSeed;
             storedBlockNumber = block.number + 1;
             seedSet = true;
         }
     
         function bet() public {
             require(!betsClosed);
             // Make bets here
         }
     
         function reveal(bytes32 _seed) public {
             require(seedSet);
             require(betMade);
             require(storedBlockNumber < block.number);
             require(keccak256(msg.sender, _seed) == sealedSeed);
             uint random = uint(keccak256(_seed, blockhash(storedBlockNumber)));
             // Insert logic for usage of random number here;
             seedSet = false;
             betsClosed = false;
         }
     }

   • Pros: Simple to implement.

   • Cons: Reveal should be done in the next 256 blocks.

Consider implementing any of the suggested alternatives to generate the random numbers.

Remediation Plan

ACKNOWLEDGED: The WinWin team acknowledged this issue and stated that the random number generation will be kept off-chain until oracle/rng becomes available in Pulsechain.