037.md

Low Tangerine Cod

High

protocol incorrectly issues abond tokens

Summary

Colors_credit incorrectly computes state values. Sum of two account normalized values should be the same before and after transfer.

Root Cause

Protocol mints bond tokens whenever borrower withdraw his deposit, I think 20% of half of his deposit would be returns as abond tokens. There will be call to

    function mintAbondToken(
        IABONDToken abond,
        uint64 bondRatio,
        address toAddress,
        uint64 index,
        uint256 amount
    ) public returns (uint128) {
...
        bool minted = abond.mint(toAddress, index, amount);
...
    }

contracts/lib/BorrowLib.sol#L605 later to _credit function

    function _credit(
        State memory _fromState,
        State memory _toState,
        uint128 _amount
    ) internal pure returns (State memory){

        // find the average cumulatie rate
        _toState.cumulativeRate = _calculateCumulativeRate(_amount, _toState.aBondBalance, _fromState.cumulativeRate, _toState.cumulativeRate);
        // find the eth backed
        _toState.ethBacked = _calculateEthBacked(_amount, _toState.aBondBalance, _fromState.ethBacked, _toState.ethBacked);
        // increment abond balance
        _toState.aBondBalance += _amount;

        return _toState;
    }

Blockchian/contracts/lib/Colors.sol#L27 But _calculateCumulativeRate, _calculateEthBacked incorrectly and computes both of them independely which breaks normalized amount that abond token should have when user will redeem using external protocol.

    function _calculateCumulativeRate(uint128 _balanceA, uint128 _balanceB, uint256 _crA, uint256 _crB) internal pure returns (uint256){
        // If balance A is zero revert
        if (_balanceA == 0) revert InsufficientBalance();
        uint256 currentCumulativeRate;
        currentCumulativeRate = ((_balanceA * _crA) + (_balanceB * _crB)) / (_balanceA + _balanceB);
        return currentCumulativeRate;
    }

    function _calculateEthBacked(uint128 _balanceA, uint128 _balanceB, uint128 _ethBackedA, uint128 _ethBackedB) internal pure returns (uint128){
        // If balance A is zero revert
        if (_balanceA == 0) revert InsufficientBalance();
        uint128 currentEthBacked;
        currentEthBacked = ((_balanceA * _ethBackedA) + (_balanceB * _ethBackedB)) / (_balanceA + _balanceB);
        return currentEthBacked;
    }

It should look like this, compute it in ETH like, normalized

$$ \frac{\text{amountFrom} \cdot \text{ethBackedFrom}}{\text{rateFrom}} + \frac{\text{amountTo} \cdot \text{ethBackedTo}}{\text{rateTo}} = \frac{(\text{amountFrom} + \text{amountTo}) \cdot \text{ethBackedWhat}}{\text{rateFrom}} $$

$$ \text{ethBackedWhat} = \frac{ \frac{\text{amountFrom} \cdot \text{ethBackedFrom}}{\text{rateFrom}} + \frac{\text{amountTo} \cdot \text{ethBackedTo}}{\text{rateTo}}}{\text{amountFrom} + \text{amountTo}} \cdot \text{rateFrom} $$

Internal pre-conditions

1. User decides to withdraw multiple times and not at once
2. User decides to transfer his abond tokens to someone else

External pre-conditions

No response

Attack Path

None, always happening, try to send not full amount abond from user to another user and back, you will see their normalized amount will be incorrect

Impact

1. Protocol issues incorrectly number abond tokens. Users will receive less or more tokens than they suppose to
2. Abond tokens lose its value on transfer. I had example transfer from user to another user, it does loses half of its value on redeeming sometimes

PoC

Insert test/foundry

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;

import {Test, StdUtils, console} from "forge-std/Test.sol";
import {ABONDToken} from "../../contracts/Token/Abond_Token.sol";
import {State, IABONDToken} from "../../contracts/interface/IAbond.sol";

contract ABONDTokenTest is Test {
//    DeployBorrowing deployer;
//    DeployBorrowing.Contracts contractsA;
//    DeployBorrowing.Contracts contractsB;

    address ethUsdPriceFeed;

    address public owner = 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266;
    address public user1 = 0x70997970C51812dc3A010C7d01b50e0d17dc79C8;
    address public user2 = 0x4d5F47FA6A74757f35C14fD3a6Ef8E3C9BC514E8;
    uint128 private PRECISION = 1e18;
    uint256 private CUMULATIVE_PRECISION;

    function test_simple() public {
        vm.startPrank(owner);
        ABONDToken token = new ABONDToken();
        token.initialize();

        token.setBorrowingContract(address(this));
        vm.startPrank(address(this));
        token.setAbondData(user1, 0, 1 ether / 2, 1111111);
        token.setAbondData(user2, 0, 1 ether / 5, 2222222);

        token.mint(user1, 0, 10000);
        token.mint(user2, 0, 10000);

        State memory state1 = IABONDToken(address(token)).userStates(user1);
        State memory state2 = IABONDToken(address(token)).userStates(user2);
//        console.log(state1.cumulativeRate, state1.ethBacked, state1.aBondBalance);
//        console.log(state2.cumulativeRate, state2.ethBacked, state2.aBondBalance);
        uint sum1 = getNormalizedAmount(state1);
        uint sum2 = getNormalizedAmount(state2);
        console.log(sum1);
        console.log(sum2);

        vm.startPrank(user1);
        token.transfer(user2, 5000);
//        vm.startPrank(user2);
//        token.transfer(user1, 5000);

        state1 = IABONDToken(address(token)).userStates(user1);
        state2 = IABONDToken(address(token)).userStates(user2);
//        console.log(state1.cumulativeRate, state1.ethBacked, state1.aBondBalance);
//        console.log(state2.cumulativeRate, state2.ethBacked, state2.aBondBalance);
        uint sum3 = getNormalizedAmount(state1);
        uint sum4 = getNormalizedAmount(state2);

        require(sum1+sum2 == sum3 +sum4);

    }

    function getNormalizedAmount(State memory userState) public returns (uint256 normalizedAmount){
        uint128 aBondAmount = userState.aBondBalance;
        uint128 depositedAmount = (aBondAmount * userState.ethBacked) / PRECISION;
        // Calculate normalized amount
        normalizedAmount = (depositedAmount) / userState.cumulativeRate;
    }

}

Mitigation

Here is an example of what formula should be, so normalized value doens't change on user transfer. THe same formula should be everwhere where _credit is used

    function transfer(
        address to,
        uint256 value
    ) public override returns (bool) {
        // check the input params are non zero
        require(msg.sender != address(0) && to != address(0), "Invalid User");

        // get the sender and receiver state
        State memory fromState = userStates[msg.sender];
        State memory toState = userStates[to];

        // check sender has enough abond to transfer
        require(fromState.aBondBalance >= value, "Insufficient aBond balance");


        toState.ethBacked = uint128((toState.aBondBalance * toState.ethBacked / toState.cumulativeRate + value * fromState.ethBacked / fromState.cumulativeRate)
        * toState.cumulativeRate / (toState.aBondBalance + value));
        toState.aBondBalance += uint128(value);

        userStates[to] = toState;

        // update sender state
        fromState = Colors._debit(fromState, uint128(value));
        userStates[msg.sender] = fromState;

        // transfer abond
        super.transfer(to, value);
        return true;
    }