sherlock-audit / 2023-05-ironbank-judging

2 stars 2 forks source link

oxchryston - The function `setcreditlimit` can be `frontruned` leading to more funds being collected. #343

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

oxchryston

medium

The function setcreditlimit can be frontruned leading to more funds being collected.

Summary

The function setcreditlimit can be frontrun to borrow above the amount intended by the creditlimitManager

Vulnerability Detail

In the contract the function setcreditlimit is used to set the credit limit of a user. if the user already has a credit limit the function can be used to increase or decrease the user's credit limit. A malicious user can frontrun this call to borrow before his credit limit is reset to a different limit which he can still borrow.

 function setCreditLimit(address user, address market, uint256 credit) external onlyCreditLimitManager {
        DataTypes.Market storage m = markets[market];
        require(m.config.isListed, "not listed");

        if (credit == 0 && creditLimits[user][market] != 0) {
            allCreditMarkets[user].deleteElement(market);
        } else if (credit != 0 && creditLimits[user][market] == 0) {
            allCreditMarkets[user].push(market);
        }

        creditLimits[user][market] = credit;
        emit CreditLimitChanged(user, market, credit);
    }

Impact

Through a sandwich attack, a malicious user can borrow to their credit limit and after the limit has been reset, borrow again, which leads to the user borrowing more funds than intended.

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/9ebf1702b2163b55479624794ab7999392367d2a/ib-v2/src/protocol/pool/IronBank.sol#L634

Tool used

Manual Review

Recommendation

use increasecreditlimit() and decreasecreditlimit() for incrementing and decrementing the credit limit of users.