code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L338-L393 https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L466-L515 https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L544-L613 https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L688-L748 https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L231-L246 https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L800-L806

Vulnerability details

Impact

Calling the following ParticleExchange.buyNftFromMarket, ParticleExchange.repayWithNft, ParticleExchange.refinanceLoan, and ParticleExchange.auctionBuyNft functions accrue interestAccrued for the corresponding lender. When calling these functions, the portions of these accrued interests that should belong to _treasury are not accrued.

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L338-L393

    function buyNftFromMarket(
        Lien calldata lien,
        ...
    ) external payable override validateLien(lien, lienId) {
        ...

        uint256 payableInterest = _calculateCurrentPayableInterest(lien);

        ...

        // accure interest to lender
        interestAccrued[lien.lender] += payableInterest;

        ...
    }

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L466-L515

    function repayWithNft(
        Lien calldata lien,
        ...
    ) external override validateLien(lien, lienId) {
        ...

        // accure interest to lender
        uint256 payableInterest = _calculateCurrentPayableInterest(lien);
        interestAccrued[lien.lender] += payableInterest;

        ...
    }

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L544-L613

    function refinanceLoan(
        Lien calldata oldLien,
        ...
    ) external override validateLien(oldLien, oldLienId) validateLien(newLien, newLienId) {
        ...
        uint256 payableInterest = _calculateCurrentPayableInterest(oldLien);
        ...

        // accure interest to the lender
        interestAccrued[oldLien.lender] += payableInterest;

        ...
    }

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L688-L748

    function auctionBuyNft(
        Lien calldata lien,
        ...
    ) external override validateLien(lien, lienId) auctionLive(lien) {
        uint256 payableInterest = _calculateCurrentPayableInterest(lien);

        ...

        // accure interest to lender
        interestAccrued[lien.lender] += payableInterest;

        ...
    }

Then, when calling the following ParticleExchange._withdrawAccountInterest function, the portion of interestAccrued[lender] that should belong to _treasury based on _treasuryRate is accrued, and the remaining portion of interestAccrued[lender] is sent to the corresponding lender.

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L231-L246

    function _withdrawAccountInterest(address payable lender) internal {
        uint256 interest = interestAccrued[lender];
        if (interest == 0) return;

        interestAccrued[lender] = 0;

        if (_treasuryRate > 0) {
            uint256 treasuryInterest = MathUtils.calculateTreasuryProportion(interest, _treasuryRate);
            _treasury += treasuryInterest;
            interest -= treasuryInterest;
        }

        lender.transfer(interest);

        emit WithdrawAccountInterest(lender, interest);
    }

However, since _treasuryRate can be updated through calling the following ParticleExchange.setTreasuryRate function, _treasuryRate can be different when interestAccrued is accrued and when interestAccrued is withdrawn. If _treasuryRate is lower when interestAccrued is accrued and is higher when interestAccrued is withdrawn, the interest received by the lender at the withdrawal time can be less than that if the interest could be withdrawn before _treasuryRate becomes higher. As a result, the lender loses some interest that it is entitled to, which is unfair to the lender.

https://github.com/code-423n4/2023-05-particle/blob/bbd1c01407a017046c86fdb483bbabfb1fb085d8/contracts/protocol/ParticleExchange.sol#L800-L806

    function setTreasuryRate(uint256 rate) external onlyOwner {
        if (rate > MathUtils._BASIS_POINTS) {
            revert Errors.InvalidParameters();
        }
        _treasuryRate = rate;
        emit UpdateTreasuryRate(rate);
    }

Proof of Concept

The following steps can occur for the described scenario.

Alice is the lender of a lien.
After the buyNftFromMarket function is called for this lien, Alice's interestAccrued becomes 0.01e18. At this moment, _treasuryRate is 1_000.
After some time, the ParticleExchange.setTreasuryRate function is called to change _treasuryRate to 5_000.
Alice calls the ParticleExchange.withdrawNftWithInterest function, which further calls the ParticleExchange._withdrawAccountInterest function, to withdraw her interest that is 0.01e18 * (1 - 5_000 / 10_000) = 0.005e18.
However, if Alice could withdraw her interest before _treasuryRate is changed, she should receive 0.01e18 * (1 - 1_000 / 10_000) = 0.009e18, which is higher than 0.005e18. Hence, Alice loses 0.009e18 - 0.005e18 = 0.004e18 in interest.

Tools Used

VSCode

Recommended Mitigation Steps

The ParticleExchange.buyNftFromMarket, ParticleExchange.repayWithNft, ParticleExchange.refinanceLoan, and ParticleExchange.auctionBuyNft functions can be respectively updated to accrue the portion of payableInterest that should belong to _treasury based on the current _treasuryRate and accrue the remaining portion of payableInterest to interestAccrued for the corresponding lender. Then, the ParticleExchange._withdrawAccountInterest function can be updated to not accrue the portion of interestAccrued[lender] for _treasury or deduct such portion from the interest payable to the lender since these actions are already done in the updated ParticleExchange.buyNftFromMarket, ParticleExchange.repayWithNft, ParticleExchange.refinanceLoan, and ParticleExchange.auctionBuyNft functions.