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 audit is an event in which community participants, referred to as Wardens, review, audit, or analyze smart contract logic in exchange for a bounty provided by sponsoring projects.

During the audit outlined in this document, C4 conducted an analysis of the Canto Invitational smart contract system written in Solidity. The audit took place between January 25—January 29 2024.

Wardens

In Code4rena’s Invitational audits, the competition is limited to a small group of wardens; for this audit, 5 wardens contributed reports:

1. bin2chen
2. xuwinnie
3. erebus
4. rvierdiiev
5. Stormy

This audit was judged by Alex the Entrprenerd.

Final report assembled by PaperParachute.

Summary

The C4 analysis yielded an aggregated total of 4 unique vulnerabilities. Of these vulnerabilities, 2 received a risk rating in the category of HIGH severity and 2 received a risk rating in the category of MEDIUM severity.

Additionally, C4 analysis included 5 reports detailing issues with a risk rating of LOW severity or non-critical.

All of the issues presented here are linked back to their original finding.

Scope

The code under review can be found within the C4 Canto Invitational repository, and is composed of 1 smart contract written in the Solidity programming language and includes 106 lines of Solidity code.

In addition to the known issues identified by the project team, an Automated Findings report was generated using the 4naly3er bot and all findings therein were classified as out of scope.

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 (2)

[H-01] update_market() market weight incorrect

Submitted by bin2chen, also found by xuwinnie

In update_market() We need to get the weight percentage of the corresponding market epoch through gaugeController.

Then allocate cantoPerBlock[epoch] according to the percentage The main logic code is as follows:

    function update_market(address _market) public {
        require(lendingMarketWhitelist[_market], "Market not whitelisted");
        MarketInfo storage market = marketInfo[_market];
        if (block.number > market.lastRewardBlock) {
            uint256 marketSupply = lendingMarketTotalBalance[_market];
            if (marketSupply > 0) {
                uint256 i = market.lastRewardBlock;
                while (i < block.number) {
                    uint256 epoch = (i / BLOCK_EPOCH) * BLOCK_EPOCH; // Rewards and voting weights are aligned on a weekly basis
                    uint256 nextEpoch = i + BLOCK_EPOCH;
                    uint256 blockDelta = Math.min(nextEpoch, block.number) - i;
                    uint256 cantoReward = (blockDelta *
                        cantoPerBlock[epoch] *
@>                      gaugeController.gauge_relative_weight_write(_market, epoch)) / 1e18;
                    market.accCantoPerShare += uint128((cantoReward * 1e18) / marketSupply);
                    market.secRewardsPerShare += uint128((blockDelta * 1e18) / marketSupply); // TODO: Scaling
                    i += blockDelta;
                }
            }
            market.lastRewardBlock = uint64(block.number);
        }
    }

1. Calculate epoch
2. Then get the corresponding weight of the market through gaugeController.gauge_relative_weight_write(market,epoch)

The problem is that epoch is block number

market.lastRewardBlock = uint64(block.number)

uint256 epoch = (i / BLOCKEPOCH) * BLOCKEPOCH

But the second parameter of gaugeController.gauge_relative_weight_write(market,epoch) is time

gauge_relative_weight_write()->_gauge_relative_weight()

contract GaugeController {
@>  uint256 public constant WEEK = 7 days;
...
    function _gauge_relative_weight(address _gauge, uint256 _time) private view returns (uint256) {
@>      uint256 t = (_time / WEEK) * WEEK;
        uint256 total_weight = points_sum[t].bias;
        if (total_weight > 0) {
            uint256 gauge_weight = points_weight[_gauge][t].bias;
            return (MULTIPLIER * gauge_weight) / total_weight;
        } else {
            return 0;
        }
    }

For example, the current canto BLOCK: 7999034 After calculation in gaugeController, it is: 7999034 / WEEK * WEEK = 1970-04-02 The wrong time cycle, the weight obtained is basically 0, so the reward cannot be obtained.

Impact

Incorrectly using block number as a time parameter, unable to get weight, unable to accumulate rewards.

Recommended Mitigation

It is recommended that LendingLedger refer to GaugeController, and also use time to record epoch.

OpenCoreCH (Canto) confirmed and commented:

True, this was time in the past, will be changed.

Alex the Entreprenerd (Judge) commented:

The finding shows how, due to using the incorrect units (blocks instead of seconds), it is possible to cause the cantoReward math to be incorrect.

Based on the block the result would either cause a total loss of rewards (example from the warden) or an incorrect amount.

While the impact is limited to rewards, the incorrect formula seems to warrant a High Severity.

---

[H-02] update_market() nextEpoch calculation incorrect

Submitted by bin2chen, also found by xuwinnie

A very important logic of update_market() is to update accCantoPerShare. When updating, if it crosses the epoch boundary, it needs to use the corresponding epoch’s cantoPerBlock[epoch]. For example: cantoPerBlock[100000] = 100 cantoPerBlock[200000] = 0 (No reward) lastRewardBlock = 199999 block.number = 200002

At this time, accCantoPerShare needs to be increased: = cantoPerBlock[100000] * 1 Delta + cantoPerBlock[200000] *2 Delta = 100 * (200000 - 199999) + 0 * (200002 - 200000) = 100

The code is as follows:

    function update_market(address _market) public {
        require(lendingMarketWhitelist[_market], "Market not whitelisted");
        MarketInfo storage market = marketInfo[_market];
        if (block.number > market.lastRewardBlock) {
            uint256 marketSupply = lendingMarketTotalBalance[_market];
            if (marketSupply > 0) {
                uint256 i = market.lastRewardBlock;
                while (i < block.number) {
                    uint256 epoch = (i / BLOCK_EPOCH) * BLOCK_EPOCH; // Rewards and voting weights are aligned on a weekly basis
@>                  uint256 nextEpoch = i + BLOCK_EPOCH;
                    uint256 blockDelta = Math.min(nextEpoch, block.number) - i;
                    uint256 cantoReward = (blockDelta *
                        cantoPerBlock[epoch] *
                        gaugeController.gauge_relative_weight_write(_market, epoch)) / 1e18;
                    market.accCantoPerShare += uint128((cantoReward * 1e18) / marketSupply);
                    market.secRewardsPerShare += uint128((blockDelta * 1e18) / marketSupply); // TODO: Scaling
                    i += blockDelta;
                }
            }
            market.lastRewardBlock = uint64(block.number);
        }
    }

The above code, the calculation of nextEpoch is wrong

uint256 nextEpoch = i + BLOCK_EPOCH;

The correct one should be

uint256 nextEpoch = epoch + BLOCK_EPOCH;

As a result, the increase of accCantoPerShare becomes: = cantoPerBlock[100000] * 3 Delta = 100 * (200002 - 199999) = 300

Impact

The calculation of update_market() is incorrect when crossing the epoch boundary, causing the calculation of accCantoPerShare to be incorrect, which may increase the allocated rewards.

Proof of Concept

The following code demonstrates the above example, when added to LendingLedgerTest.t.sol:

    function test_ErrorNextEpoch() external {
        vm.roll(fromEpoch);        
        whiteListMarket();
        //only set first , second cantoPerBlock = 0
        vm.prank(goverance);
        ledger.setRewards(fromEpoch, fromEpoch, amountPerBlock);
        vm.prank(lendingMarket);
        ledger.sync_ledger(lender, 1e18);
        
        //set lastRewardBlock = nextEpoch - 10
        vm.roll(fromEpoch + BLOCK_EPOCH - 10);
        ledger.update_market(lendingMarket);

        //cross-border
        vm.roll(fromEpoch + BLOCK_EPOCH + 10);
        ledger.update_market(lendingMarket);

        //show more
        (uint128 accCantoPerShare,,) = ledger.marketInfo(lendingMarket);
        console.log("accCantoPerShare:",accCantoPerShare);
        console.log("more:",accCantoPerShare - amountPerEpoch);

    }

$ forge test -vvv --match-test test_ErrorNextEpoch

Running 1 test for src/test/LendingLedger.t.sol:LendingLedgerTest
[PASS] test_ErrorNextEpoch() (gas: 185201)
Logs:
  accCantoPerShare: 1000100000000000000
  more: 100000000000000

Recommended Mitigation

    function update_market(address _market) public {
        require(lendingMarketWhitelist[_market], "Market not whitelisted");
        MarketInfo storage market = marketInfo[_market];
        if (block.number > market.lastRewardBlock) {
            uint256 marketSupply = lendingMarketTotalBalance[_market];
            if (marketSupply > 0) {
                uint256 i = market.lastRewardBlock;
                while (i < block.number) {
                    uint256 epoch = (i / BLOCK_EPOCH) * BLOCK_EPOCH; // Rewards and voting weights are aligned on a weekly basis
-                   uint256 nextEpoch = i + BLOCK_EPOCH;
+                   uint256 nextEpoch = epoch + BLOCK_EPOCH;

Alex the Entreprenerd (Judge) commented:

The Warden has shown a mistake in accounting, due to relying on a spot time, it is possible to influence the math in a way that would result in incorrect accounting.

Because of this I agree with High Severity.

OpenCoreCH (Canto) confirmed

---

Medium Risk Findings (2)

[M-01] secRewardsPerShare Insufficient precision

Submitted by bin2chen, also found by xuwinnie

We also introduced the field secRewardDebt. The idea of this field is to enable any lending platforms that are integrated with Neofinance Coordinator to send their own rewards based on this value (or rather the difference of this value since the last time secondary rewards were sent) and their own emission schedule for the tokens.

The current calculation formula for secRewardsPerShare is as follows:

market.secRewardsPerShare += uint128((blockDelta * 1e18) / marketSupply);

marketSupply is cNOTE, with a precision of 1e18 So as long as the supply is greater than 1 cNote, secRewardsPerShare is easily rounded down to 0 Example: marketSupply = 10e18 blockDelta = 1 secRewardsPerShare=1 * 1e18 / 10e18 = 0

Impact

Due to insufficient precision, secRewardsPerShare will basically be 0.

Recommended Mitigation

It is recommended to use 1e27 for secRewardsPerShare:

    market.secRewardsPerShare += uint128((blockDelta * 1e27) / marketSupply);

Alex the Entreprenerd (Judge) commented:

The Warden has shown how, due to incorrect precision, it is possible to create a loss due to rounding down.

Because the loss is limited to yield, Medium Severity seems most appropriate.

OpenCoreCH (sponsor) confirmed

---

[M-02] Loss of precission when calculating the accumulated CANTO per share

Submitted by erebus

When calculating the amount of CANTO per share in update_market, dividing by 1e18 in cantoReward and multiplying by the same value in accCantoPerShare rounds down the final value, making the amount of rewards users will receive be less than expected.

Proof of Concept

It’s well known that Solidity rounds down when doing an integer division, and because of that, it is always recommended to multiply before dividing to avoid that precision loss. However, if we go to:

LendingLedger, function update_market

    function update_market(address _market) public {
        require(lendingMarketWhitelist[_market], "Market not whitelisted");
        MarketInfo storage market = marketInfo[_market];
        if (block.number > market.lastRewardBlock) {
            uint256 marketSupply = lendingMarketTotalBalance[_market];
            if (marketSupply > 0) {
                uint256 i = market.lastRewardBlock;
                while (i < block.number) {
                    uint256 epoch = (i / BLOCK_EPOCH) * BLOCK_EPOCH; // Rewards and voting weights are aligned on a weekly basis
                    uint256 nextEpoch = i + BLOCK_EPOCH;
                    uint256 blockDelta = Math.min(nextEpoch, block.number) - i;
                    uint256 cantoReward = (blockDelta *
                        cantoPerBlock[epoch] *
                        gaugeController.gauge_relative_weight_write(_market, epoch)) / 1e18;
                    market.accCantoPerShare += uint128((cantoReward * 1e18) / marketSupply);
                    market.secRewardsPerShare += uint128((blockDelta * 1e18) / marketSupply); // TODO: Scaling
                    i += blockDelta;
                }
            }
            market.lastRewardBlock = uint64(block.number);
        }
    }

and we expand the maths behind accCantoPerShare and cantoReward:

                    uint256 cantoReward = (blockDelta *
                        cantoPerBlock[epoch] *
                        gaugeController.gauge_relative_weight_write(_market, epoch)) / 1e18;
                    market.accCantoPerShare += uint128((cantoReward * 1e18) / marketSupply);

like follows:

$(cantoReward \* 1e18) \ / \ marketSupply$

$(((blockDelta \* cantoPerBlock\[epoch] \* gaugeController.gauge_relative_weight_write(\_market, epoch)) \ / \ 1e18) \ \* \ 1e18) \ / \ marketSupply$

$((stuff \ / \ 1e18) \ \* \ 1e18) \ / \ marketSupply$

we see there is a hidden division before a multiplication that is rounding down the whole expression. This is bad as the precision loss can be significant, which leads to the given market having less rewards to offer to its users. Run the next test inside a foundry project to see such a divergence in the precision if we multiply before dividing:

    // @audit the assumes are to avoid under/overflows errors
    function testPOC(uint256 x) external pure {
        vm.assume(x < type(uint256).max / 1e18);
        vm.assume(x > 1e18);
        console2.log("(x / 1e18) * 1e18", (x / 1e18) * 1e18);
        console2.log("(x * 1e18) / 1e18", (x * 1e18) / 1e18);
    }

Some examples:

  [7725] POC::testPOC(1164518589284217277370 [1.164e21]) 
    ├─ [0] VM::assume(true) [staticcall]
    │   └─ ← ()
    ├─ [0] VM::assume(true) [staticcall]
    │   └─ ← ()
    ├─ [0] console::log((x / 1e18) * 1e18, 1164000000000000000000 [1.164e21]) [staticcall]
    │   └─ ← ()
    ├─ [0] console::log((x * 1e18) / 1e18, 1164518589284217277370 [1.164e21]) [staticcall]
    │   └─ ← ()
    └─ ← ()

  [7725] POC::testPOC(16826228168456047587 [1.682e19]) 
    ├─ [0] VM::assume(true) [staticcall]
    │   └─ ← ()
    ├─ [0] VM::assume(true) [staticcall]
    │   └─ ← ()
    ├─ [0] console::log((x / 1e18) * 1e18, 16000000000000000000 [1.6e19]) [staticcall]
    │   └─ ← ()
    ├─ [0] console::log((x * 1e18) / 1e18, 16826228168456047587 [1.682e19]) [staticcall]
    │   └─ ← ()
    └─ ← ()

  [7725] POC::testPOC(5693222591586418917 [5.693e18]) 
    ├─ [0] VM::assume(true) [staticcall]
    │   └─ ← ()
    ├─ [0] VM::assume(true) [staticcall]
    │   └─ ← ()
    ├─ [0] console::log((x / 1e18) * 1e18, 5000000000000000000 [5e18]) [staticcall]
    │   └─ ← ()
    ├─ [0] console::log((x * 1e18) / 1e18, 5693222591586418917 [5.693e18]) [staticcall]
    │   └─ ← ()
    └─ ← ()

Recommended Mitigation Steps

Change the update_market function to:

    function update_market(address _market) public {
        require(lendingMarketWhitelist[_market], "Market not whitelisted");
        MarketInfo storage market = marketInfo[_market];
        if (block.number > market.lastRewardBlock) {
            uint256 marketSupply = lendingMarketTotalBalance[_market];
            if (marketSupply > 0) {
                uint256 i = market.lastRewardBlock;
                while (i < block.number) {
                    uint256 epoch = (i / BLOCK_EPOCH) * BLOCK_EPOCH; // Rewards and voting weights are aligned on a weekly basis
                    uint256 nextEpoch = i + BLOCK_EPOCH;
                    uint256 blockDelta = Math.min(nextEpoch, block.number) - i;
                    uint256 cantoReward = (blockDelta *
                        cantoPerBlock[epoch] *
-                       gaugeController.gauge_relative_weight_write(_market, epoch)) / 1e18;
-                   market.accCantoPerShare += uint128((cantoReward * 1e18) / marketSupply);
+                       gaugeController.gauge_relative_weight_write(_market, epoch)) * 1e18;
+                   market.accCantoPerShare += uint128((cantoReward / 1e18) / marketSupply);
                    market.secRewardsPerShare += uint128((blockDelta * 1e18) / marketSupply); // TODO: Scaling
                    i += blockDelta;
                }
            }
            market.lastRewardBlock = uint64(block.number);
        }
    }

OpenCoreCH (Canto) confirmed and commented:

The given examples do not directly apply in our case, we divide a number with 36 decimals by 1e18 and then multiply by 1e18 again (blockDelta has 0, cantoPerBlock 18, gauge_relative_weight_write 18). Looking at it again, this is pretty inefficient, but I cannot think of a situation where it causes a non-negligible (higher than a few wei) difference in the end. Will double check that.

Ok so I think it could lead to situations where up to 1 CANTO is not distributed, e.g. if (blockDelta * cantoPerBlock[epoch] * gaugeController.gauge_relative_weight_write(_market, epoch)) is 0.99e18, it will round down to 0. Not a huge loss, but also not ideal (especially because it can be avoided easily), so will be changed.

Alex the Entreprenerd (Judge) commented:

The Warden has shown how, due to rounding a small loss of yield can accrue.

Due to the small scope of the codebase and the specificity of the finding, I’m leaving as Medium as of now.

OpenCoreCH (Canto) commented

---

Low Risk and Non-Critical Issues

For this audit, 5 reports were submitted by wardens detailing low risk and non-critical issues. The report highlighted below by rvierdiiev received the top score from the judge.

The following wardens also submitted reports: erebus, bin2chen, Stormy, and xuwinnie.

[L-01] setRewards allows to set rewards for epochs in the past

Description

LendingLedger.setRewards accepts _fromEpoch and _toEpoch params. The only check that function does is if epoch are set as weeks. But there is no check if rewards are set for previous epochs.

Recommendation

Check that provided epochs are in future

[L-02] If setRewards will be called for same epochs, then cantoPerBlock will be rewritten

Description

LendingLedger.setRewards accepts _fromEpoch and _toEpoch params. The only check that function does is if epoch are set as weeks. It allows to provide epochs that were already set before. In case if this will happen, which is likely not the case as sponsor claimed, then cantoPerBlock will be rewritten for the epoch.

Recommendation

Increase cantoPerBlock for the epoch. cantoPerBlock[i] += _amountPerBlock;

[L-03] Loss of rewards for users in case if market is blacklisted

Description Issue 1

In case if market was whitelisted, earned rewards and then became blacklisted, then it will be not possible for users to claim already earned rewards anymore, because update_market function revert for non whitelisted markets.

Recommendation Issue 1

Give ability for users to claim already accrued rewards for blacklisted market. Maybe you should remove require in the update_market function. As i guess that blacklisted market will have 0 weight in gauge. Then make sure sync_ledger checks that msg.sender is whitelisted market.

Description Issue 2

Any market can be blacklisted. This means that rewards will not accrue for it anymore.

When market is blacklisted, then update_market is not called for it for last time(and already accrued rewards are not available for users as i described in QA-04). And those undistributed earned rewards are not tracked in any way to be distributed to other markets. Thus, those funds will just stuck.

Recommendation Issue 2

You need to allow user to claim all funds(and when blacklisting you should call update_market for last time) or you need to track unclaimed funds and redistribute them other markets.

[R-01] Contract doesn’t emit any event

Description

LendingLedger doesn’t emit any event which can help tracking activity for offline services. For example when market is whitelisted/blacklisted, when rewards per block is set for new epochs, when user claimed rewards or when governance has changed.

Recommendation

Add events.

GalloDaSballo commented:

QA-01 setRewards allows to set rewards for epochs in the past
L

QA-02 If setRewards will be called for same epochs, then cantoPerBlock will be rewritten
L

QA-03 Contract doesn’t emit any event
R

QA-04 Loss of rewards for users in case if market is blacklisted
L

QA-05 Some rewards will be lost in case if market will be blacklisted
See QA-04

3L 1R

C4 Note: QA-04 and QA-05 have been merged into L-03

OpenCoreCH (sponsor) confirmed

---

Audit Analysis

For this audit, 1 analysis reports were submitted by wardens. An analysis report examines the codebase as a whole, providing observations and advice on such topics as architecture, mechanism, or approach. The report highlighted below by rvierdiiev received the top score from the judge.

Overview

Canto protocol has redesigned LendingLedger contract to allow users to claim their rewards at any time, so they should not wait when epoch will finish. Also secRewardDebt variable was introduced to allow third parties to distribute rewards to users based on changes of that value during the time.

Key Components

LendingLedger contract allows governance to set rewards for the epochs, which should be distributed among whitelisted markets according to their weight in gauge controller. LendingLedger contract tracks total supply of each whitelisted market in order to calculate correct reward per share for the users. Market’s participants can receive rewards according to the amount of shares that they have in the market. Also each user has secRewardDebt variable that is calculated to allow third parties to rewards users.

Systemic Risks

Correct decisions from governance: whitelisted markets and rewards per epoch are set by governance. Thus governance is responsible to validate provided values as it possible to break intended work with incorrect input.

Centralization risk

LendingLedger has only governance role, which should belong to the DAO. No other party is allowed to change contract settings.

Time spent:

4 hours

---

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.