search

sherlock-audit / 2024-03-zivoe-judging

8 stars 6 forks source link

Bauchibred - Protocol's functionality can end up being broken based on the asset being used #699

Closed sherlock-admin4 closed 6 months ago

sherlock-admin4 commented 6 months ago

Bauchibred

high

Protocol's functionality can end up being broken based on the asset being used

Summary

Protocol's functionality can be broken as users could be forced to pay worse rates as interest due to the actions of restricted admins of some assets being integrated

Vulnerability Detail

First note that from the contest's readMe, the two facts below has been stated

### Q: In case of external protocol integrations, are the risks of external contracts pausing or executing an emergency withdrawal acceptable? If not, Watsons will submit issues related to these situations that can harm your protocol's functionality.

Not acceptable

---

And also from https://github.com/sherlock-audit/2024-03-zivoe/blob/d4111645b19a1ad3ccc899bea073b6f19be04ccd/README.md#L40-L42 the below has been stated in regards to external admins protocol integrate with

### Q: Are the admins of the protocols your contracts integrate with (if any) TRUSTED or RESTRICTED?

RESTRICTED

---

Now take a look at https://github.com/sherlock-audit/2024-03-zivoe/blob/d4111645b19a1ad3ccc899bea073b6f19be04ccd/zivoe-core-foundry/src/lockers/OCC/OCC_Modular.sol#L434-L457

    function amountOwed(uint256 id) public view returns (
        uint256 principal, uint256 interest, uint256 lateFee, uint256 total
    ) {
        // 0 == Bullet.
        if (loans[id].paymentSchedule == 0) {
            if (loans[id].paymentsRemaining == 1) { principal = loans[id].principalOwed; }
        }
        // 1 == Amortization (only two options, use else here).
        else { principal = loans[id].principalOwed / loans[id].paymentsRemaining; }

        // Add late fee if past loans[id].paymentDueBy.
        if (block.timestamp > loans[id].paymentDueBy && loans[id].state == LoanState.Active) {
            lateFee = loans[id].principalOwed * (block.timestamp - loans[id].paymentDueBy) *
                loans[id].APRLateFee / (86400 * 365 * BIPS);
        }
        interest = loans[id].principalOwed * loans[id].paymentInterval * loans[id].APR / (86400 * 365 * BIPS);
        total = principal + interest + lateFee;
    }

This function returns the information for amount owed on next payment of a particular loan which the data returned from this is heavily used in other instances of codes, would be key to note that for any OCC_Modular contract, the stable coin to be used is passed in the constructor.

Stablecoins to be used in protocol as hinted in the readMe include USDC/USDT

Going to the implementations of these contracts on etherscan, from https://etherscan.io/address/0x43506849d7c04f9138d1a2050bbf3a0c054402dd#code & https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7#code# we can see that their admins both have the functionality of pausing the contracts, or blacklisting any address that integrates with this or even toggling the fee on transfer mechanism in the case of USDT.

All these three functionalities could heavily affect protocol, for example considering on sherlock the readMe is the primary source of truth the admin being restricted means that blacklisting issues are now valid and the admin of the stablecoins could blacklist all address of Zivoe to break access to core functionalities present in protocol.

Alternatively, the admins can pause the transfers making core functions including acceptOffer, callLoan, makePayment, supplyInterest unavailable, This could then subtly cause written loans to be untakeable or payments to not be possible and other effects on the users in regards to the interests.

Impact

Protocol's core functionality is broken and would have an effect on users

Code Snippet

https://github.com/sherlock-audit/2024-03-zivoe/blob/d4111645b19a1ad3ccc899bea073b6f19be04ccd/zivoe-core-foundry/src/lockers/OCC/OCC_Modular.sol#L434-L457

Tool used

Manual Code Review

Recommendation

Ensure that the protocol team and its users are aware of the risks of such an event and develop a contingency plan to manage it.

Duplicate of #657

pseudonaut commented 6 months ago

I suppose valid, not of concern

sherlock-admin4 commented 6 months ago

1 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, there is nothing can be done in such case, so it's just informational issue