Farseen0/scar-pairs huggingface dataset:
MEDIUM severity: Borrowing from branches can be disabled by one whale or early depositor. **Impact**

Major changes in V2 include:
- 72% of interest raid paid out to SP stakers
- Inability to borrow new debt in RM

The logic can be seen in `_requireValidAdjustmentInCurrentMode\
https://github.com/liquity/bold/blob/a34960222df5061fa7c0213df5d20626adf3ecc4/contracts/src/BorrowerOperations....

Vulnerable code:
function _requireValidAdjustmentInCurrentMode(
        TroveChange memory _troveChange,
        LocalVariables_adjustTrove memory _vars
    ) internal view {
        /*
        * Below Critical Threshold, it is not permitted:
        *
        * - Borrowing
        * - Collateral withdrawal except accompanied by a debt repayment of at least the same value
        *
        * In Normal Mode, ensure:
        *
        * - The adjustment won't pull the TCR below CCR
        *
        * In Both cases:
        * - The new ICR is above MCR
        */
        _requireICRisAboveMCR(_vars.newICR);

        if (_vars.isBelowCriticalThreshold) {
            _requireNoBorrowing(_troveChange.debtIncrease);
            _requireDebtRepaymentGeCollWithdrawal(_troveChange, _vars.price);
        } else {
            // if Normal Mode
            uint256 newTCR = _getNewTCRFromTroveChange(_troveChange, _vars.price);
            _requireNewTCRisAboveCCR(newTCR);
        }
    }
MEDIUM severity: Zappers and swaps can leave dust. **Executive Summary**

Zapper can leave dust amounts under two conditions:
- Debt to repay is less than the amount -> Dust amount, QA in most cases
- Borrowed amount is more than enough to repay the flashloan -> Will result in a significant amount of WETH being stuck

The code snippets are listed...

Vulnerable code:
uint256 receivedBoldAmount =
            exchange.swapToBold(_effectiveFlashLoanAmount, _params.minBoldAmount, address(this));

        // Adjust trove
        borrowerOperations.adjustTrove(
            _params.troveId,
            _params.flashLoanAmount,
            false, // _isCollIncrease
            receivedBoldAmount, /// @audit could cause dust
            false, // _isDebtIncrease
            0
        );

// When the adjustment is a debt repayment, check it's a valid amount and that the caller has enough Bold
        if (_troveChange.debtDecrease > 0) {
            uint256 maxRepayment = vars.trove.entireDebt > MIN_DEBT ? vars.trove.entireDebt - MIN_DEBT : 0;
            if (_troveChange.debtDecrease > maxRepayment) {
                _troveChange.debtDecrease = maxRepayment;
            }
            _requireSufficientBoldBalance(vars.boldToken, msg.sender, _troveChange.debtDecrease);
        }

borrowerOperations.adjustTrove(
            _params.troveId,
            _effectiveFlashLoanAmount, // flash loan amount minus fee
            true, // _isCollIncrease
            _params.boldAmount,
            true, // _isDebtIncrease
            _params.maxUpfrontFee
        );

        // Swap Bold to Coll
        // No need to use a min: if the obtained amount is not enough, the flash loan return below won’t be enough
        // And the flash loan provider will revert after this function exits
        // The frontend should calculate in advance the `_params.boldAmount` needed for this to work
        exchange.swapFromBold(_params.boldAmount, _params.flashLoanAmount, address(this));
LOW severity: Best to assert oracle decimals in constructor. **Impact**

https://github.com/liquity/bold/blob/a34960222df5061fa7c0213df5d20626adf3ecc4/contracts/src/PriceFeeds/CompositePriceFeed.sol#L34-L36

Chainlink price feeds tend to have either 8 or 18 decimals

As to ensure that the decimals are the intended amount, you could add a simple assert in...

Vulnerable code:
lstEthOracle.aggregator = AggregatorV3Interface(_lstEthOracleAddress);
        lstEthOracle.stalenessThreshold = _lstEthStalenessThreshold;
        lstEthOracle.decimals = lstEthOracle.aggregator.decimals();
LOW severity: Unclear comment around managers. **Impact**

The code in `_openTrove` is as follows:

https://github.com/liquity/bold/blob/a34960222df5061fa7c0213df5d20626adf3ecc4/contracts/src/BorrowerOperations.sol#L360-L365

Which asserts that the manager condition is maintained

It is unclear which condition should be maintained

as of...

Vulnerable code:
// TODO: We can restore the condition for non-zero managers if we end up ipmlementing at least one of:
        // - wipe them out on closing troves
        // - do not reuse troveIds
        // for now it is safer to make sure they are set
        _setAddManager(vars.troveId, _addManager);
        _setRemoveManagerAndReceiver(vars.troveId, _removeManager, _receiver);
LOW severity: Rounding in favour of users will cause slight inaccuracies over time. **Impact**

Liquity V2 rounds in favour of the user in a few spots

-> Total Debt
-> Reduces the average debt weight -> Charges slightly less over time

-> Batch Debt
-> Increases the total debt sum
-> Charges slightly higher over time

**QA - `aggBatchManagementFees` individual user discount will.....

Vulnerable code:
_latestTroveData.accruedBatchManagementFee =
                _latestBatchData.accruedManagementFee * batchDebtShares / totalDebtShares;

return Math.ceilDiv(aggWeightedDebtSum * (block.timestamp - lastAggUpdateTime), ONE_YEAR * DECIMAL_PRECISION);
LOW severity: TroveManager can make troves liquidatable by changing the batch interest rate. **Impact**

This is a finding to make a clear known risk more obvious

Users that add their Trove to a Batch are allowing the BatchManager to charge a lot of fees by simply adjusting the interest rate as soon as they can via...

Vulnerable code:
// Apply upfront fee on premature adjustments
        if (
            batch.annualInterestRate != _newAnnualInterestRate
                && block.timestamp < batch.lastInterestRateAdjTime + INTEREST_RATE_ADJ_COOLDOWN
        ) {
            uint256 price = _requireOraclesLive();

            uint256 avgInterestRate = activePoolCached.getNewApproxAvgInterestRateFromTroveChange(batchChange);
            batchChange.upfrontFee = _calcUpfrontFee(newDebt, avgInterestRate);
            _requireUserAcceptsUpfrontFee(batchChange.upfrontFee, _maxUpfrontFee);

            newDebt += batchChange.upfrontFee;

            // Recalculate the batch's weighted terms, now taking into account the upfront fee
            batchChange.newWeightedRecordedDebt = newDebt * _newAnnualInterestRate;
            batchChange.newWeightedRecordedBatchManagementFee = newDebt * batch.annualManagementFee;

            // Disallow a premature adjustment if it would result in TCR < CCR
            // (which includes the case when TCR is already below CCR before the adjustment).
            uint256 newTCR = _getNewTCRFromTroveChange(batchChange, price);
            _requireNewTCRisAboveCCR(newTCR);
        }
LOW severity: Trove Adjustments may be griefed by sandwhich raising the average interest rate. **Impact**

BO operations require accepting an upfront fee

https://github.com/liquity/bold/blob/3ad11270a22190e77c1e8ef7742d2ebec133a317/contracts/src/BorrowerOperations.sol#L334-L335

This is effectively a percentage of the debt change (not necessarily of TCR due to price changes)

Due to this,...

Vulnerable code:
_requireUserAcceptsUpfrontFee(_change.upfrontFee, _maxUpfrontFee);
LOW severity: Old Comments. https://github.com/liquity/bold/blob/3ad11270a22190e77c1e8ef7742d2ebec133a317/contracts/src/TroveManager.sol#L690-L711

https://github.com/liquity/bold/blob/3ad11270a22190e77c1e8ef7742d2ebec133a317/contracts/src/ActivePool.sol#L39

https://github.com/liquity/bold/blob/3ad11270a22190e77c1e8ef7742d2eb...

Vulnerable code:
/* Send _boldamount Bold to the system and redeem the corresponding amount of collateral from as many Troves as are needed to fill the redemption
    * request.  Applies redistribution gains to a Trove before reducing its debt and coll.
    *
    * Note that if _amount is very large, this function can run out of gas, specially if traversed troves are small. This can be easily avoided by
    * splitting the total _amount in appropriate chunks and calling the function multiple times.
    *
    * Param `_maxIterations` can also be provided, so the loop through Troves is capped (if it’s zero, it will be ignored).This makes it easier to
    * avoid OOG for the frontend, as only knowing approximately the average cost of an iteration is enough, without needing to know the “topology”
    * of the trove list. It also avoids the need to set the cap in stone in the contract, nor doing gas calculations, as both gas price and opcode
    * costs can vary.
    *
    * All Troves that are redeemed from -- with the likely exception of the last one -- will end up with no debt left, therefore they will be closed.
    * If the last Trove does have some remaining debt, it has a finite ICR, and the reinsertion could be anywhere in the list, therefore it requires a hint.
    * A frontend should use getRedemptionHints() to calculate what the ICR of this Trove will be after redemption, and pass a hint for its position
    * in the sortedTroves list along with the ICR value that the hint was found for.
    *
    * If another transaction modifies the list between calling getRedemptionHints() and passing the hints to redeemCollateral(), it
    * is very likely that the last (partially) redeemed Trove would end up with a different ICR than what the hint is for. In this case the
    * redemption will stop after the last completely redeemed Trove and the sender will keep the remaining Bold amount, which they can attempt
    * to redeem later.
    */
    function redeemCollateral(

uint256 internal collBalance; // deposited ether tracker

uint256 internal collBalance; // deposited ether tracker
LOW severity: Unbackedness is a bit of an inaccurate way to reduce risk via redemptions. https://github.com/liquity/bold/blob/57c2f13fe78f914cab3d9736f73b4e0ffc58e022/contracts/src/CollateralRegistry.sol#L122-L131

First of all:
- A whale can manipulate the unbackedness at any time
- Unbackedness is not a true measure of liquidation risk, TCR is

Lower TCR branches would benefit more...

Vulnerable code:
for (uint256 index = 0; index < totals.numCollaterals; index++) {
            ITroveManager troveManager = getTroveManager(index);
            (uint256 unbackedPortion, uint256 price, bool redeemable) =
                troveManager.getUnbackedPortionPriceAndRedeemability();
            prices[index] = price;
            if (redeemable) {
                totals.unbacked += unbackedPortion;
                unbackedPortions[index] = unbackedPortion;
            }
        }
LOW severity: QA - Base Rate Decay may be slower than intended - Chaduke. **Note**
This is an already known finding from ETHOS (Liquity Fork)

**Impact**

`_calcDecayedBaseRate` calls `_minutesPassedSinceLastFeeOp` which rounds down by up to 1 minute - 1

https://github.com/liquity/bold/blob/57c2f13fe78f914cab3d9736f73b4e0ffc58e022/contracts/src/CollateralRegistry.sol#L22...

Vulnerable code:
function _calcDecayedBaseRate() internal view returns (uint256) {
        uint256 minutesPassed = _minutesPassedSinceLastFeeOp();
        uint256 decayFactor = LiquityMath._decPow(REDEMPTION_MINUTE_DECAY_FACTOR, minutesPassed);

        return baseRate * decayFactor / DECIMAL_PRECISION;
    }

function _updateLastFeeOpTime() internal {
        uint256 timePassed = block.timestamp - lastFeeOperationTime;

        if (timePassed >= ONE_MINUTE) {
            lastFeeOperationTime = block.timestamp;
            emit LastFeeOpTimeUpdated(block.timestamp);
        }
    }

    function _minutesPassedSinceLastFeeOp() internal view returns (uint256) {
        return (block.timestamp - lastFeeOperationTime) / ONE_MINUTE;
    }

    // Updates the `baseRate` state with math from `_getUpdatedBaseRateFromRedemption`
    function _updateBaseRateAndGetRedemptionRate(uint256 _boldAmount, uint256 _totalBoldSupplyAtStart) internal {
        uint256
LOW severity: Rounding Error for Batch in Trove can be used to flag a Trove as Unredeemable while having MIN_DEBT or more. **Impact**

The code for handling `unredeemable` troves after a redemption is as follows:

https://github.com/liquity/bold/blob/6959eb503d12ad9b30bbdb787e220c1efbd5d761/contracts/src/TroveManager.sol#L664-L674

Where `newDebt` is computed in this...

Vulnerable code:
uint256 newDebt = _applySingleRedemption(_defaultPool, _singleRedemption, isTroveInBatch);

        // Make Trove unredeemable if it's tiny, in order to prevent griefing future (normal, sequential) redemptions
        if (newDebt < MIN_DEBT) {
            Troves[_singleRedemption.troveId].status = Status.unredeemable;
            if (isTroveInBatch) {
                sortedTroves.removeFromBatch(_singleRedemption.troveId);
            } else {
                sortedTroves.remove(_singleRedemption.troveId);
            }
        }

uint256 newDebt = _singleRedemption.trove.entireDebt - _singleRedemption.boldLot;
LOW severity: `weightedRecordedDebt` update logic sometimes uses relative incorrect ratios while the absolute values are correct. **Executive Summary**

Some changes to `weightedRecordedDebt`use relative values which are incorrect but the absolute changes are correct when updating a Trove in a Batch

**Impact**

When updating a Trove in a batch. the `batch.entireDebtWithoutRedistribution` already includes the Trove debt...

Vulnerable code:
if (isTroveInBatch) {
            singleLiquidation.oldWeightedRecordedDebt = /// @audit adding the debt again
                batch.weightedRecordedDebt + (trove.entireDebt - trove.redistBoldDebtGain) * batch.annualInterestRate;
            singleLiquidation.newWeightedRecordedDebt = batch.entireDebtWithoutRedistribution * batch.annualInterestRate; /// @audit not removing the debt
            // Mint batch management fee
            troveChange.batchAccruedManagementFee = batch.accruedManagementFee;
            troveChange.oldWeightedRecordedBatchManagementFee = batch.weightedRecordedBatchManagementFee
                + (trove.entireDebt - trove.redistBoldDebtGain) * batch.annualManagementFee;
            troveChange.newWeightedRecordedBatchManagementFee =
                batch.entireDebtWithoutRedistribution * batch.annualManagementFee;
            activePool.mintBatchManagementFeeAndAccountForChange(troveChange, batchAddress);
        } else {

if (_isTroveInBatch) {
            _getLatestBatchData(_singleRedemption.batchAddress, _singleRedemption.batch);
            // We know boldLot <= trove entire debt, so this subtraction is safe
            uint256 newAmountForWeightedDebt = _singleRedemption.batch.entireDebtWithoutRedistribution
                + _singleRedemption.trove.redistBoldDebtGain - _singleRedemption.boldLot;
            _singleRedemption.oldWeightedRecordedDebt = _singleRedemption.batch.weightedRecordedDebt;
            _singleRedemption.newWeightedRecordedDebt =
                newAmountForWeightedDebt * _singleRedemption.batch.annualInterestRate;

uint256 newAggBatchManagementFees = aggBatchManagementFees; // 1 SLOAD
        newAggBatchManagementFees += calcPendingAggBatchManagementFee();
        newAggBatchManagementFees -= _troveChange.batchAccruedManagementFee;
        aggBatchManagementFees = newAggBatchManagementFees; // 1 SSTORE

        // Do the arithmetic in 2 steps here to avoid overflow from the decrease
        uint256 newAggWeightedBatchManagementFeeSum = aggWeightedBatchManagementFeeSum; // 1 SLOAD
        newAggWeightedBatchManagementFeeSum += _troveChange.newWeightedRecordedBatchManagementFee;
        newAggWeightedBatchManagementFeeSum -= _troveChange.oldWeightedRecordedBatchManagementFee;
        aggWeightedBatchManagementFeeSum = newAggWeightedBatchManagementFeeSum; // 1 SSTORE
LOW severity: Urgent Redemptions Premium can worsen the ICR when Trove Coll Value < Debt Value * .1. **Summary**

The math for Redemptions is sound and generally speaking Liquidations are always more profitable hence preferred

It's worth noting that once the value of Debt is sufficiently high, redemptions can worsen the TCR

The following python scripts shows a scenario:

In this instance COLL...

Vulnerable code:
BOLD = 100
COLL = 100.9
PREMIUM_BPS = 100 ## 1%
MAX_BPS = 10_000


"""
  Redeem 10
  Gives 11

  Bold = 90
  Coll = 99

  Redeem 10
  Gives 11

  Bold = 80
  Coll = 88

  Bold = 70
  Coll = 77


"""

print("Initial CR", COLL / BOLD)

while(BOLD > 0):
  BOLD -= 10
  COLL -= (10 + 10 * PREMIUM_BPS / MAX_BPS)

  print("BOLD", BOLD)
  print("COLL", COLL)
  print("CR", COLL / BOLD)

Initial CR 1.0090000000000001
BOLD 90
COLL 90.80000000000001
CR 1.008888888888889
BOLD 80
COLL 80.70000000000002
CR 1.0087500000000003
BOLD 70
COLL 70.60000000000002
CR 1.008571428571429
BOLD 60
COLL 60.50000000000002
CR 1.0083333333333337
BOLD 50
COLL 50.40000000000002
CR 1.0080000000000005
BOLD 40
COLL 40.30000000000002
CR 1.0075000000000005
BOLD 30
COLL 30.200000000000017
CR 1.0066666666666673
BOLD 20
COLL 20.100000000000016
CR 1.0050000000000008
BOLD 10
COLL 10.000000000000016
CR 1.0000000000000016
BOLD 0
COLL -0.09999999999998366
Traceback (most recent call last):
  File "/Users/entreprenerd/Desktop/Consulting/bold/premium.py", line 34, in <module>
    print("CR", COLL / BOLD)
                ~~~~~^~~~~~
LOW severity: Mathematical Reasoning around Rounding Errors and their Impacts for Troves in Batches. **Impact**

This finding summarizes my research on how Truncation could be abused, the goal is to define the mechanisms and quantify the impacts

**Minting of Batch Shares**

Minting (and burning) of batch shares boils down to this...

Vulnerable code:
batchDebtSharesDelta = currentBatchDebtShares * debtIncrease / _batchDebt;

_latestTroveData.recordedDebt = _latestBatchData.recordedDebt * batchDebtShares / totalDebtShares;
LOW severity: Add and Remove Managers may open up to phishing. **Impact**

A Phishing scam can be performed on Troves by selling a trove and removing all collateral as the RemoveManager

https://github.com/liquity/bold/blob/57c2f13fe78f914cab3d9736f73b4e0ffc58e022/contracts/src/Dependencies/AddRemoveManagers.sol#L48-L59

I don't really like these type of...

Vulnerable code:
function setAddManager(uint256 _troveId, address _manager) external {
        _requireCallerIsBorrower(_troveId);
        _setAddManager(_troveId, _manager);
    }

    function _setAddManager(uint256 _troveId, address _manager) internal {
        addManagerOf[_troveId] = _manager;
    }

    function setRemoveManager(uint256 _troveId, address _manager) external {
        setRemoveManagerWithReceiver(_troveId, _manager, troveNFT.ownerOf(_troveId));
    }
LOW severity: One Year is 365.25 days. https://github.com/liquity/bold/blob/c84585881d8c6a5d11d38deee2943ba949a39962/contracts/src/Dependencies/Constants.sol#L46-L47

Can be changed to 365.25 to be more accurate with what a year is in solar terms

This can also be nofixed with the caveat that technically the system is slightly...

Vulnerable code:
uint256 constant ONE_YEAR = 365 days;
LOW severity: `TroveNFT.tokenURI` `debt` and `coll` are static and will not reflect interest and redistributions. **Impact**

`TroveNFT.tokenURI` is as follows:

https://github.com/liquity/bold/blob/c84585881d8c6a5d11d38deee2943ba949a39962/contracts/src/TroveNFT.sol#L33-L48

Which is not using `getLatestTroveData` 

This means that the data displayed in the NFT will not be updated until a Trove is accrued...

Vulnerable code:
function tokenURI(uint256 _tokenId) public view override(ERC721, IERC721Metadata) returns (string memory) {
        (uint256 debt, uint256 coll,, ITroveManager.Status status,,,, uint256 annualInterestRate,,) =
            troveManager.Troves(_tokenId);

        IMetadataNFT.TroveData memory troveData = IMetadataNFT.TroveData({
            _tokenId: _tokenId,
            _owner: ownerOf(_tokenId),
            _collToken: address(collToken),
            _collAmount: coll,
            _debtAmount: debt,
            _interestRate: annualInterestRate,
            _status: status
        });

        return metadataNFT.uri(troveData);
    }
LOW severity: Zappers would benefit by capping the amount of bold to repay to the current Trove debt amount. **Impact**

`Zappers._adjustTrovePre` performs the following operation:

https://github.com/liquity/bold/blob/c84585881d8c6a5d11d38deee2943ba949a39962/contracts/src/Zappers/WETHZapper.sol#L197-L199

This amount may be higher than the Trove Debt

Meaning the tokens will be stuck in the...

Vulnerable code:
if (!_isDebtIncrease) { /// @audit should be capped just like adjust
            boldToken.transferFrom(msg.sender, address(this), _boldChange);
        }
LOW severity: User provided Zapper could be used to skim leftovers. **Impact**

Both UniV3 and Curve Swappers look as follows:

https://github.com/liquity/bold/blob/c84585881d8c6a5d11d38deee2943ba949a39962/contracts/src/Zappers/Modules/Exchanges/CurveExchange.sol#L58-L83

They receive the zapper from which to swap from as a parameter

This can be used to sweep out.....

Vulnerable code:
function swapFromBold(uint256 _boldAmount, uint256 _minCollAmount, address _zapper) external returns (uint256) {
        ICurvePool curvePoolCached = curvePool;
        IBoldToken boldTokenCached = boldToken;
        boldTokenCached.transferFrom(_zapper, address(this), _boldAmount);
        boldTokenCached.approve(address(curvePoolCached), _boldAmount);

        // TODO: make this work
        //return curvePoolCached.exchange(BOLD_TOKEN_INDEX, COLL_TOKEN_INDEX, _boldAmount, _minCollAmount, false, _zapper);
        uint256 output = curvePoolCached.exchange(BOLD_TOKEN_INDEX, COLL_TOKEN_INDEX, _boldAmount, _minCollAmount);
        collToken.safeTransfer(_zapper, output);

        return output;
    }

    function swapToBold(uint256 _collAmount, uint256 _minBoldAmount, address _zapper) external returns (uint256) {
        ICurvePool curvePoolCached = curvePool;
        IERC20 collTokenCached = collToken;
        collTokenCached.safeTransferFrom(_zapper, address(this), _collAmount);
        collTokenCached.approve(address(curvePoolCached), _collAmount);

        //return curvePoolCached.exchange(COLL_TOKEN_INDEX, BOLD_TOKEN_INDEX, _collAmount, _minBoldAmount, false, _zapper);
        uint256 output = curvePoolCached.exchange(COLL_TOKEN_INDEX, BOLD_TOKEN_INDEX, _collAmount, _minBoldAmount);
        boldToken.transfer(_zapper, output); /// @audit is `exchange` correct?

        return output;
    }
LOW severity: `DefaultPool` Typo. **Impact**

The coll is no longer `ether`

https://github.com/liquity/bold/blob/e9cc36dc4a53eed2336113095eea93989cbdac3a/contracts/src/DefaultPool.sol#L53-L60

Vulnerable code:
/*
    * Returns the collBalance state variable.
    *
    * Not necessarily equal to the the contract's raw Coll balance - ether can be forcibly sent to contracts.
    */
    function getCollBalance() external view override returns (uint256) {
        return collBalance;
    }
LOW severity: `AddressRegistry` can benefit by having some small additional sanitization. **Impact**

The `AddressRegistry` acts as configuration tool for the rest of the system

A few key variables are checked for absolute bounds, but there's no check for the relation between them

https://github.com/liquity/bold/blob/e9cc36dc4a53eed2336113095eea93989cbdac3a/contracts/src/AddressesRegis...

Vulnerable code:
constructor(
        address _owner,
        uint256 _ccr,
        uint256 _mcr,
        uint256 _scr,
        uint256 _liquidationPenaltySP,
        uint256 _liquidationPenaltyRedistribution
    ) Ownable(_owner) {
        if (_ccr <= 1e18 || _ccr >= 2e18) revert InvalidCCR();
        if (_mcr <= 1e18 || _mcr >= 2e18) revert InvalidMCR();
        if (_scr <= 1e18 || _scr >= 2e18) revert InvalidSCR();
        if (_liquidationPenaltySP < 5e16) revert SPPenaltyTooLow();
        if (_liquidationPenaltySP > _liquidationPenaltyRedistribution) revert SPPenaltyGtRedist();
        if (_liquidationPenaltyRedistribution > 10e16) revert RedistPenaltyTooHigh();

        CCR = _ccr;
        SCR = _scr;
        MCR = _mcr;
        LIQUIDATION_PENALTY_SP = _liquidationPenaltySP;
        LIQUIDATION_PENALTY_REDISTRIBUTION = _liquidationPenaltyRedistribution;
    }