Canto Application Specific Dollars and Bonding Curves for 1155s - ether_sky's results

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/asD/src/asD.sol#L73-L77

Vulnerability details

Impact

The owner of an asD token can only withdraw carry. To facilitate this, we computed the maximum available carry amount that the owner can withdraw. However, this calculated value should be divided by 1e18 not 1e28. Consequently, our calculation yields an incorrect carry amount—significantly smaller than the actual amount. As a result, the owner can withdraw only a small portion, or the transaction may revert due to underflow.

Proof of Concept

I have reviewed the CNote tokens. The hierarchy is as follows:

contract CNote is CErc20Delegate {}
contract CErc20Delegate is CErc20, CDelegateInterface {}
contract CErc20 is CToken, CErc20Interface {}
abstract contract CToken is CTokenInterface, ExponentialNoError, TokenErrorReporter {}

The scale factor expScale value is defined as a constant, and its value is set to 1e18.

contract ExponentialNoError {
    uint constant expScale = 1e18;
}

When attempting to retrieve the exchangeRateCurrent value, the following function in the CToken will be invoked:

function exchangeRateCurrent() public override nonReentrant returns (uint) {
    accrueInterest();
    return exchangeRateStored();
}
function exchangeRateStored() public view override returns (uint) {
    return exchangeRateStoredInternal();
}

Consequently, this will trigger the exchangeRateStoredInternal function in CNote.

function exchangeRateStoredInternal() internal view virtual override returns (uint256) {
    uint256 _totalSupply = totalSupply;
    if (_totalSupply == 0) {
        return initialExchangeRateMantissa;
    } else {
        uint256 cashPlusBorrowsMinusReserves = totalBorrows - totalReserves; // totalCash in cNote Lending Market is zero, thus it is not factored into the exchangeRate
        uint256 exchangeRate = (cashPlusBorrowsMinusReserves * expScale) / _totalSupply;

        return exchangeRate;
    }
}

Here, the _totalSupply value is denominated in cToken's decimals, while cashPlusBorrowsMinusReserves is denominated in underlying token's decimals. We can track these values in the following functions:

function mintFresh(address minter, uint mintAmount) internal virtual {
    ...
    totalSupply = totalSupply + mintTokens;
}
function borrowFresh(address payable borrower, uint borrowAmount) internal virtual {
    ...
    uint totalBorrowsNew = totalBorrows + borrowAmount;
    totalBorrows = totalBorrowsNew;
}

The decimals for all cTokens are 8, and the decimal for our note token is 18. Let me provide an example. Suppose the cNote token balance is 1e8 (equivalent to 1 CNote), and the underlying token balance is 1e18 (equivalent to 1 Note). In this case, the exchangeRateCurrent value is calculated as 1e28 (which equals 1e18 * expScale / 1e8). In our withdrawCarry function, we can obtain this value from the following line:

uint256 exchangeRate = CTokenInterface(cNote).exchangeRateCurrent();

Then (CTokenInterface(cNote).balanceOf(address(this)) * exchangeRate) / 1e28 = 1e8 * 1e28 / 1e28 = 1e8. However, the actual balance of the underlying token is 1e18.

I believe the meaning of Scaled by 1 * 10^(18 - 8 + Underlying Token Decimals), i.e., 10^(28) in our case is as follows:

• 1e18 is the scale factor.
• (Underlying Token Decimal - 8) is related to the decimals of totalBorrows and totalSupply.

In short, all calculations to convert between cToken and underlying tokens are based on 1e18(expScale). i.e.

underlying token balance = cToken balance * exchangeRateCurrent / 1e18

Furthermore, we can confirm this by examining the balanceOfUnderlying function.

function balanceOfUnderlying(
    address owner
) external override returns (uint) {
    Exp memory exchangeRate = Exp({mantissa: exchangeRateCurrent()});
    return mul_ScalarTruncate(exchangeRate, accountTokens[owner]);
}
function mul_ScalarTruncate(Exp memory a, uint scalar) internal pure returns (uint) {
    Exp memory product = mul_(a, scalar);
    return truncate(product);
}
function mul_(Exp memory a, uint b) internal pure returns (Exp memory) {
    return Exp({mantissa: mul_(a.mantissa, b)});
}
function mul_(uint a, uint b) internal pure returns (uint) {
    return a * b;
}
function truncate(Exp memory exp) internal pure returns (uint) {
    // Note: We are not using careful math here as we're performing a division that cannot fail
    return exp.mantissa / expScale;
}

Tools Used

Recommended Mitigation Steps

Please modify withdrawCarry function as follows:

function withdrawCarry(uint256 _amount) external onlyOwner {
    - uint256 exchangeRate = CTokenInterface(cNote).exchangeRateCurrent();         
    - uint256 maximumWithdrawable = (CTokenInterface(cNote).balanceOf(address(this)) * exchangeRate) /
    -     1e28 -
    -     totalSupply();
    
    + uint256 maximumWithdrawable = CTokenInterface(cNote).balanceOfUnderlying(address(this));
}

or divide by 1e18 instead of 1e28.

Assessed type

Decimal

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/1155tech-contracts/src/Market.sol#L158

Vulnerability details

Impact

Based on the system design, users are unable to claim fees associated with this purchase when attempting to buy tokens. So, these fees should be distributed among other holders, the platform, and the creator of that share. However, a portion will be stuck in the market, and nobody can utilize it due to the incorrect implementation of the fee split logic.

Proof of Concept

To prevent users from claiming fees from their new purchase, the unclaimed fees are calculated for that user before splitting the new fee. After splitting this new fee, the user is then updated that there are no pending fees anymore.

uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);

// Split the fee among holder, creator and platform
_splitFees(_id, fee, shareData[_id].tokensInCirculation);
rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;

SafeERC20.safeTransfer(token, msg.sender, rewardsSinceLastClaim);

The new fee is divided among token holders based on the number of tokens in circulation, represented by shareData[_id].tokensInCirculation.

_splitFees(_id, fee, shareData[_id].tokensInCirculation);

function _splitFees(uint256 _id, uint256 _fee, uint256 _tokenCount) internal {
    if (_tokenCount > 0) {
        shareData[_id].shareHolderRewardsPerTokenScaled += (shareHolderFee * 1e18) / _tokenCount;
    }
}

However, this value includes the tokens held by the current user. Consequently, a portion is allocated to that user if their token count is non-zero. But the rewardsLastClaimedValue variable of that user is updated with the latest shareHolderRewardsPerTokenScaled value without claiming this fee.

_splitFees(_id, fee, shareData[_id].tokensInCirculation);
rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;

As a result, these fees become stuck in the market.

Please add below test to Market.t.sol.

    function testBut2Times() public {
        testCreateNewShare();
        token.approve(address(market), 1e18);

        uint256 price1 = LINEAR_INCREASE;
        uint256 fee1 = LINEAR_INCREASE / 10;
        market.buy(1, 1);
        assertEq(token.balanceOf(address(market)), price1 + fee1);

        uint256 price2 = 2 * LINEAR_INCREASE;
        uint256 fee2 = (2 * LINEAR_INCREASE) / 10;
        market.buy(1, 1);
        assertEq(token.balanceOf(address(market)), price1 + fee1 + price2 + fee2);

        uint256 balBefore = token.balanceOf(address(this));
        uint256 balBobBefore = token.balanceOf(address(bob));
        uint256 creatorFee = (fee1 * 33) / 100 + (fee2 * 33) / 100;
        uint256 platformFee = (fee1 * 67) / 100 + (fee2 * 34) / 100;
        uint256 holderFee = 0;
        uint256 remainingFee = fee1 + fee2 - creatorFee - platformFee; // = fee2 * 33 / 100

        vm.prank(bob);
        market.claimCreatorFee(1);
        assertEq(token.balanceOf(address(bob)), balBobBefore + creatorFee);

        vm.prank(address(this));
        market.claimPlatformFee();
        assertEq(token.balanceOf(address(address(this))), balBefore + platformFee);
        market.claimHolderFee(1);
        assertEq(token.balanceOf(address(address(this))), balBefore + platformFee + holderFee);

        // The remaining fee is currently stuck in the market, and no one can utilize it.
        assertEq(token.balanceOf(address(market)), price1 + price2 + remainingFee);
    }

Tools Used

Recommended Mitigation Steps

Please modify buy function as below:

function buy(uint256 _id, uint256 _amount) external {
    - _splitFees(_id, fee, shareData[_id].tokensInCirculation);
    + _splitFees(_id, fee, shareData[_id].tokensInCirculation - tokensByAddress[_id][msg.sender]);
}

Assessed type

Error