Security Audit Report for Cakepie Contracts

Report Manifest

Version History

1. Introduction

1.1 About Target Contracts

The target of this audit is the code repositiory of CakePie Contracts^1 of Magpie XYZ. The CakePie Contracts runs a CakeRush campaign that users can convert their CAKE token or locked CAKE position from PancakeSwap on CakePie. Please note that only CakeRush.sol and PancakeStakingBNBChain.sol are included in the audit scope, while other files are out of scope for this audit.

The auditing process is iterative. Specifically, we would audit the commits that fix the discovered issues. If there are new issues, we will continue this process. The commit SHA values during the audit are shown in the following table. Our audit report is responsible for the code in the initial version (Version1), as well as new code (in the following versions) to fix issues in the audit report.

1.2 Security Model

To evaluate the risk, we follow the standards or suggestions that are widely adopted by both industry and academy, including OWASP Risk Rating Methodology ^2 and Common Weakness Enumeration ^3. The overall severity of the risk is determined by likelihood and impact. Specifically, likelihood is used to estimate how likely a particular vulnerability can be uncovered and exploited by an attacker, while impact is used to measure the consequences of a successful exploit.

In this report, both likelihood and impact are categorized into two ratings, i.e., high and low respectively, and their combinations are shown in Table 1.1.

Accordingly, the severity measured in this report are classified into three categories: High, Medium, Low. For the sake of completeness, Undetermined is also used to cover circumstances when the risk cannot be well determined.

Furthermore, the status of a discovered item will fall into one of the following four categories:

• Undetermined No response yet.

• Acknowledged The item has been received by the client, but not confirmed yet.

• Confirmed The item has been recognized by the client, but not fixed yet.

• Fixed The item has been confirmed and fixed by the client.

2. Findings

In total, we find two potential issues. Besides, we also have three recommendations and one note.

• High Risk: 1

• Low Risk: 1

• Recommendation: 3

• Note: 1

The details are provided in the following sections.

2.1 Software Security

2.1.1 Potential inconsistent state after parameter reset

Description The CakeRush contract distributes rewards according to several parameters. The following functions allows the project maintainer to reset some of the parameters:

function resetMultiplier() external onlyOwner {
        uint256 len = rewardMultiplier.length;
        for (uint8 i = 0; i < len; ++i) {
            rewardMultiplier.pop();
            rewardTier.pop();
        }

        tierLength = 0;
    }

    function resetTimeWeighting() external onlyOwner {
        uint256 len = weightedTime.length;
        for (uint8 i = 0; i < len; ++i) {
            weightedTime.pop();
            weighting.pop();
        }

        weightLength = 0;
    }

Listing 2.1: CakeRush.sol

However, these functions only reset the parameters, but not the user information stored in the userInfos state variable. As a result, calculations in the CakeRush contract can fail due to the inconsistent state. For example, if the parameters are reset and set to incorrect values, the subtraction on Line 155 can fail due to integer underflow.

function quoteConvert(
        uint256 _amountToConvert,
        address _account
    )
        external
        view
        returns (
            uint256 newUserFactor,
            uint256 newTotalFactor,
            uint256 newUserWeightedFactor,
            uint256 newWeightedTotalFactor
        )
    {
        if (_amountToConvert == 0 || rewardMultiplier.length == 0 || weighting.length == 0)
            return (0, 0, 0, 0);

        UserInfo storage userInfo = userInfos[_account];
        uint256 accumulated = _amountToConvert + userInfo.converted;

        uint256 factorAccuNoWeighting = 0;
        uint256 i = 1;
        while (i < rewardTier.length && accumulated > rewardTier[i]) {
            factorAccuNoWeighting += (rewardTier[i] - rewardTier[i - 1]) * rewardMultiplier[i - 1];
            i++;
        }
        factorAccuNoWeighting += (accumulated - rewardTier[i - 1]) * rewardMultiplier[i - 1];

        uint256 factorToEarnNoWeighting = (factorAccuNoWeighting / DENOMINATOR) - userInfo.factor;

Listing 2.2: CakeRush.sol

What's worse, if users call convert or convertWithCakePool immediately after resetting the parameters (before the new parameters are set, for example through back-running) can lead to the reset of the total and weighted factors recorded inside the contract, due to the logic on Line 141-142.

Impact Resetting parameters can lead to inconsistent and incorrect state.

Suggestion Set the new parameters after clearing the old ones.

Feedback from the Project Multipliers won't be reset once the cake rush campaign starts.

2.1.2 Repeated claim of mCake rewards

Description After locking CAKE tokens in the contract, users can claim mCake tokens as rewards through the function. However, the function contains a issue that allows the users to claim the rewards multiple times. In the following code segment, if the amount is larger than the of the user, would transfer or deposit a total amount of to the user. The correct implementation should only return , so the current implementation effectively allows a user to repeatedly claim the mCake rewards.

function claim(bool _isStake) external nonReentrant {
        UserInfo storage userInfo = userInfos[msg.sender];
        if (claimedMCake[msg.sender] >= userInfo.converted) revert AlreadyClaimed();
        if (_isStake && userInfo.converted > 0) {
            if (masterCakepie == address(0)) revert MasterCakepieNotSet();
            IERC20(mCakeOFT).safeApprove(address(masterCakepie), userInfo.converted);
            IMasterCakepie(masterCakepie).depositFor(
                address(mCakeOFT),
                address(msg.sender),
                userInfo.converted
            );
        } else if (userInfo.converted > 0) {
            IERC20(mCakeOFT).transfer(msg.sender, userInfo.converted);
            emit Claim(msg.sender, userInfo.converted);
        }

        claimedMCake[msg.sender] = userInfo.converted;
    }

Listing 2.3: CakeRush.sol

Impact Users are able to repeatedly claim mCake rewards.

Suggestion Revise the reward claim logic.

2.2 Additional Recommendation

2.2.1 Check the parameters in initialization functions

Description In the initialization functions of the CakeRush and PancakeStakingBNBChain contracts, there are parameters that cannot be changed after initialization. It is recommended that these parameters should be checked in the initialization functions.

function __CakeRush_init(
        address _cake,
        address _mCakeOFT,
        address _masterCakepie
    ) public initializer {
        __Ownable_init();
        __ReentrancyGuard_init();
        __Pausable_init();
        cake = _cake;
        mCakeOFT = _mCakeOFT;
        masterCakepie = _masterCakepie;
    }

Listing 2.4: CakeRush.sol

Impact N/A

Suggestion Check parameters in initialization functions.

2.2.2 Check the parameters in CakeRush contracts

Description In the CakeRush contract, several parameters regarding the reward distribution can be added. However, there is no check that these parameters are set correctly according to the assumptions in the contract. Specifically, in setMultipler and setTimeWeighting function, extra conditions must be checked (i.e. the monotonic increasing property of the rewardTier and weightedTime array).

function setMultiplier(
        uint256[] calldata _multiplier,
        uint256[] calldata _tier
    ) external onlyOwner {
        if (_multiplier.length == 0 || (_multiplier.length != _tier.length)) revert LengthInvalid();

        for (uint8 i = 0; i < _multiplier.length; ++i) {
            if (_multiplier[i] == 0) revert InvalidAmount();
            rewardMultiplier.push(_multiplier[i]);
            rewardTier.push(_tier[i]);
            tierLength += 1;
        }
    }

Listing 2.5: CakeRush.sol

Impact N/A

Suggestion Check parameters in the functions that set the parameters.

2.2.3 Extra conditions in modifiers

Description In the CakeRush contract, the _onlyPancakeStaking modifier has an extraneous condition. According to the semantics of this modifier, checking msg.sender != pancakeStaking would be sufficient.

modifier _onlyPancakeStaking() {
        if (pancakeStaking == address(0) || msg.sender != pancakeStaking)
            revert OnlyPancakeStaking();
        _;
    }

Listing 2.6: CakeRush.sol

Impact N/A

Suggestion Remove extraneous conditions in the modifier.

2.3 Note

2.3.1 Potential centralization risk

Description The owner of CakeRush holds significant privileges to modify critical configurations. This creates a single point of failure. If an attacker were to compromise the owner, they could potentially incapacitate the entire system.

Additionally, the CAKE tokens in the contract are not explicitly handled to lock them into the VECake contract. Instead, CakeRush allows the owner to withdraw all those CAKEs, which means the owner must lock the CAKE tokens after withdrawing. However, the logic is not guaranteed at the code level, which also brings centralization concerns.

Feedback from the Project The team make the owner as a multisig to mitigate the risk.

3. Notices and Remarks

3.1 Disclaimer

This audit report does not constitute investment advice or a personal recommendation. It does not consider, and should not be interpreted as considering or having any bearing on, the potential economics of a token, token sale or any other product, service or other asset. Any entity should not rely on this report in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset.

This audit report is not an endorsement of any particular project or team, and the report does not guarantee the security of any particular project. This audit does not give any warranties on discovering all security issues of the smart contracts, i.e., the evaluation result does not guarantee the nonexistence of any further findings of security issues. As one audit cannot be considered comprehensive, we always recommend proceeding with independent audits and a public bug bounty program to ensure the security of smart contracts.

The scope of this audit is limited to the code mentioned in Section 1.1. Unless explicitly specified, the security of the language itself (e.g., the solidity language), the underlying compiling toolchain and the computing infrastructure are out of the scope.

3.2 Procedure of Auditing

We perform the audit according to the following procedure.

• Vulnerability Detection We first scan smart contracts with automatic code analyzers, and then manually verify (reject or confirm) the issues reported by them.

• Semantic Analysis We study the business logic of smart contracts and conduct further investigation on the possible vulnerabilities using an automatic fuzzing tool (developed by our research team). We also manually analyze possible attack scenarios with independent auditors to cross-check the result.

• Recommendation We provide some useful advice to developers from the perspective of good programming practice, including gas optimization, code style, and etc.

We show the main concrete checkpoints in the following.

3.2.1 Software Security

• Reentrancy

• DoS

• Access control

• Data handling and data flow

• Exception handling

• Untrusted external call and control flow

• Initialization consistency

• Events operation

• Error-prone randomness

• Improper use of the proxy system

3.2.2 DeFi Security

• Semantic consistency

• Functionality consistency

• Permission management

• Business logic

• Token operation

• Emergency mechanism

• Oracle security

• Whitelist and blacklist

• Economic impact

• Batch transfer

3.2.3 NFT Security

• Duplicated item

• Verification of the token receiver

• Off-chain metadata security

3.2.4 Additional Recommendation

• Gas optimization

• Code quality and style

Note The previous checkpoints are the main ones. We may use more checkpoints during the auditing process according to the functionality of the project.