Playfi Pro League
Findings & Analysis Report

2024-07-29

Table of contents

Overview

About C4

Code4rena (C4) is an open organization consisting of security researchers, auditors, developers, and individuals with domain expertise in smart contracts.

A C4 Pro League Audit is an event where elite tier Code4rena contributors, commonly referred to as wardens, reviews, audits and analyzes smart contract logic in exchange for a bounty provided by sponsoring projects.

During the Pro League audit outlined in this document, C4 conducted an analysis of the Playfi smart contract system written in Solidity. The audit took place between July 2 - July 05, 2024.

Wardens

2 Wardens contributed to Playfi:

  1. 10xhash
  2. zzykxx

Final report assembled by bytes032, thebrittfactor & Sentinel

Summary

The C4 Pro League analysis yielded 3 HIGH, 1 MEDIUM and 1 LOW severity vulnerabilities.

Additionally, C4 Pro League analysis included 1 finding rated as Informational.

Scope

The source code was delivered to Code4rena in a public Git repository.

Severity Criteria

C4 assesses the severity of disclosed vulnerabilities based on three primary risk categories: high, medium, and low/non-critical.

High-level considerations for vulnerabilities span the following key areas when conducting assessments:

  • Malicious Input Handling
  • Escalation of privileges
  • Arithmetic
  • Gas use

For more information regarding the severity criteria referenced throughout the submission review process, please refer to the documentation provided on the C4 website, specifically our section on Severity Categorization.

High Risk Findings (3)

[H-1] The functions claimLicensePartner() and claimLicensePublic() dont update state variables allowing users to purchase more licenses than expected

Lines of Code

Description

The functions claimLicensePartner() and claimLicensePublic() use the variables partnerClaimsPerTierPerAddress and claimsPerTierPerAddress respectively to ensure that individual caps are respected, reverting if an user attempts to purchase more licenses than allowed.

Both variables will always 0 because their values are never updated by the new purchased licenses. This makes it possible for users to purchase more licenses than they should.

Recommendation

  • In claimLicensePartner() increase the variable partnerClaimsPerTierPerAddress[][][] by the amount of licenses purchased
  • In claimLicensePublic() increase the variable claimsPerTierPerAddress[][] by the amount of licenses purchased

Playfi

Fixed with PR-6.

C4 Pro League

Fix reviewed and confirmed.

[H-2] Users can bypass limits and purchase more licenses than allowed by re-entering functions

Lines of Code

Description

The PlayFiLicenseSale contract sends a commission in ETH to a referral address if a referral code is used during license purchases. This is generally done via a low-level call performed before state variables are updated, which allows for re-entrancy in order to bypass limits and purchase more licenses than allowed.

We will take the claimLicensePublic() function as an example:

function claimLicensePublic(uint256 amount, uint256 tier, string memory referral) public payable {
    if(!publicSaleActive) revert PublicSaleNotActive();
    if(tiers[tier].totalClaimed + amount > tiers[tier].totalCap) revert TotalTierCapExceeded();
    if(claimsPerTierPerAddress[tier][msg.sender] + amount > tiers[tier].individualCap) revert IndividualTierCapExceeded();
    (uint256 toPay, uint256 commission,) = paymentDetailsForReferral(amount, tier, referral, false);
    if(msg.value < toPay) revert InsufficientPayment();
    if(commission > 0) {
        (bool sent, ) = payable(referrals[referral].receiver).call{ value: commission }("");
        if (!sent) revert CommissionPayoutFailed();
        emit CommissionPaid(referral, referrals[referral].receiver, commission);
    }
    tiers[tier].totalClaimed += amount;
    publicClaimsPerAddress[msg.sender] += amount;
    totalLicenses += amount;
    referrals[referral].totalClaims += amount;
    emit PublicLicensesClaimed(msg.sender, amount, tier, toPay, referral);
}

Alice wants to purchase 4 licenses, but only 2 are left. As a workaround, she can:

  1. Create and deploy malicious contract MAL.sol that triggers a call to claimLicensePublic() to purchase licenses whenever ETH are received.
  2. Call the function setReferral() from MAL.sol in order to create a referral code MAL where the receiver is MAL.sol.
  3. Call the function claimLicensePublic() by using MAL as referral code and purchasing the 2 licenses left.
  4. MAL.sol will receive ETH for the commission and will internally call the function again claimLicensePublic() by purchasing 2 extra linceses. This is possible because the state variables have not been updated yet.

Recommendation

Either:

  • Use nonReentrant modifiers on all impacted functions.
  • Transfer the referral/partners commissions at the end of each function, after the state variables have been updated.

Playfi

Fixed with PR-6.

C4 Pro League

Fix reviewed and confirmed.

[H-3] Excess payment is not reimbursed causing users to spend more than necessary for the license

Lines of Code

Description

When the payment made by user’s is in excess, the remaining ETH is not returned.

    function claimLicensePublic(uint256 amount, uint256 tier, string memory referral) public payable {

        ......

        (uint256 toPay, uint256 commission,) = paymentDetailsForReferral(amount, tier, referral, false);
        if(msg.value < toPay) revert InsufficientPayment();

Since the payment amount is dependent on the referral discount which in turn is dependent on the claimCount, the actual price at the time of execution can end up being lower than at the start of the block. Hence, a user can end up making excess payment which will not be refunded.

    function paymentDetailsForReferral(uint256 amount, uint256 tier, string memory referral, bool isWhitelist) public view returns (uint256 toPay, uint256 commission, uint256 discount) {

        .......

        if(referrals[referral].receiver != address(0)) {
=>       uint256 totalClaims = referrals[referral].totalClaims;
            if(totalClaims < 20) {
                commission = fullPrice * 10 / 100;
            } else if (totalClaims < 40) {
                commission = fullPrice * 11 / 100;

Recommendation

Refund excess payment.

Edge case: This has been fixed for the functions that price licenses dynamically but there is an edge case about the claimLicenseFriendsFamily and claimLicenseEarlyAccess functions, they both use tiers[1].price as the license price which can be changed at any point by the admins via setTiers().

Playfi

Recommendation fixed with PR-6.

Fix for the edge case applied here: PlayFi-Labs/node-license-sale-contracts @3cd84fe.

C4 Pro League

Fix reviewed and confirmed.

Medium Risk Findings (1)

[M-1] Reorgs may cause licenses to be sold at 0 price

Lines of Code

Description

The payment amount for claimLicenseEarlyAccess and claimLicenseFriendsFamily is determined by tiers[1].price while the ability to start claiming is dependent on other variables like earlyAccessSaleActive and earlyAccessMerkleRoot and these are set by different entities

    function claimLicenseEarlyAccess(uint256 amount, bytes calldata data, bytes32[] calldata merkleProof) public payable {
        if(!earlyAccessSaleActive) revert EarlyAccessSaleNotActive();
        (uint256 index, uint256 claimCap) = abi.decode(data,(uint256,uint256));
        uint256 claimedLicenses = earlyAccessClaimsPerAddress[msg.sender];
        if(amount + claimedLicenses > claimCap) revert IndividualClaimCapExceeded();
        bytes32 node = keccak256(abi.encodePacked(index, msg.sender, claimCap));
        if (!MerkleProof.verify(merkleProof, earlyAccessMerkleRoot, node)) revert InvalidProof();
        uint256 toPay = tiers[1].price * amount;

The code is also planned to be deployed on Polygon where reorgs are common. This creates a possible scenario where the tier price update transaction gets pushed to a later block and a user claim transaction occurs earlier. This will cause the license to be sold at 0 price.

Recommendation

Ensure sufficient time interval (at least >1min) between the tier setting tx and the status update transaction / check if tier[1].price is zero and revert.

Playfi

Fixed with PR-6.

C4 Pro League

Fix reviewed and confirmed.

Low Risk Findings (1)

[L-1] Reorgs can cause referral code and associated fees to be assigned to another user

Lines of Code

Description

The receiver of the referral commission is solely determined the referral code which is served on a first come basis.

    function setReferral(string memory code) public {
        _setReferral(code, msg.sender);
    }

The project also plans to deploy on Polygon where reorgs are common. This allows for a scenario where the referral code is obtained by another user and the commission payments still happen.

    function claimLicensePublic(uint256 amount, uint256 tier, string memory referral) public payable {

Reorg scenario:

User A claims referral code A
User B also makes a tx to obtain referral code A which fails initially
Associated user's with A claims license with referral code A
Due to reorg, user B's tx occur earlier than user A's tx
The commission payments from the user's go to user B instead of A

Recommendation

Ensure sufficient time interval (at least >1min) between the user’s claim tx’s and the referral code setting.

Playfi

Won’t fix as UX for users will become worse and the Polygon sale will be limited in scale.

C4 Pro League

Acknowledged.

Informational Findings (1)

[I-1] getTier handles isWhitelist incorrectly

Lines of Code

Description

The getTier function doesn’t handle the isWhitelist variable correctly and returns the whitelistTier instead of normal tier when isWhitelist is false and vice-versa.

    function getTier(uint256 id, bool isWhitelist) public view returns(Tier memory tier) {
        if(isWhitelist) {
            tier = tiers[id];
        } else {
            tier = whitelistTiers[id];
        }
    }

Recommendation

Apply the following change:

     function getTier(uint256 id, bool isWhitelist) public view returns(Tier memory tier) {
-        if(isWhitelist) {
+        if(!isWhitelist) {
             tier = tiers[id];
         } else {
             tier = whitelistTiers[id];

Playfi

Fixed with PR-6.

C4 Pro League

Fix reviewed and acknowledged.

Disclosures

C4 is an open organization governed by participants in the community.

C4 audits incentivize the discovery of exploits, vulnerabilities, and bugs in smart contracts. Security researchers are rewarded at an increasing rate for finding higher-risk issues. Audit submissions are judged by a knowledgeable security researcher and solidity developer and disclosed to sponsoring developers. C4 does not conduct formal verification regarding the provided code but instead provides final verification.

C4 does not provide any guarantee or warranty regarding the security of this project. All smart contract software should be used at the sole risk and responsibility of users.