Skip to content
This repository has been archived by the owner on Apr 28, 2024. It is now read-only.

0xkaden - HPB may be incorrectly bankrupt due to use of unscaled value in _forgiveBadDebt #25

Open
sherlock-admin2 opened this issue Oct 27, 2023 · 2 comments
Labels
Medium A valid Medium severity issue Reward A payout will be made for this issue Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Oct 27, 2023

0xkaden

medium

HPB may be incorrectly bankrupt due to use of unscaled value in _forgiveBadDebt

Summary

An unscaled value is used in place of where a scaled value should be used in the bankruptcy check in _forgiveBadDebt. This may cause the bucket to be incorrectly marked as bankrupt, losing user funds.

Vulnerability Detail

At the end of _forgiveBadDebt, we do a usual bankruptcy check in which we check whether the remaining deposit and collateral will be little enough that the exchange rate will round to 0, in which case we mark the bucket as bankrupt, setting the bucket lps and effectively all user lps as 0.

The problem lies in the fact that we use depositRemaining as part of this check, which represents an unscaled value. As a result, when computing whether the exchange rate rounds to 0 our logic is off by a factor of the bucket's scale. The bucket may be incorrectly marked as bankrupt if the unscaled depositRemaining would result in an exchange rate of 0 when the scaled depositRemaining would not.

Impact

Loss of user funds.

Code Snippet

https://github.com/sherlock-audit/2023-09-ajna/blob/main/ajna-core/src/libraries/external/SettlerActions.sol#L485

// If the remaining deposit and resulting bucket collateral is so small that the exchange rate
// rounds to 0, then bankrupt the bucket.  Note that lhs are WADs, so the
// quantity is naturally 1e18 times larger than the actual product
// @audit depositRemaining should be a scaled value
if (depositRemaining * Maths.WAD + hpbBucket.collateral * _priceAt(index) <= bucketLP) {
    // existing LP for the bucket shall become unclaimable
    hpbBucket.lps            = 0;
    hpbBucket.bankruptcyTime = block.timestamp;

    emit BucketBankruptcy(
        index,
        bucketLP
    );
}

Tool used

Manual Review

Recommendation

Simply scale depositRemaining before doing the bankruptcy check, e.g. something like:

depositRemaining = Maths.wmul(depositRemaining, scale);
if (depositRemaining * Maths.WAD + hpbBucket.collateral * _priceAt(index) <= bucketLP) {
    // existing LP for the bucket shall become unclaimable
    hpbBucket.lps            = 0;
    hpbBucket.bankruptcyTime = block.timestamp;

    emit BucketBankruptcy(
        index,
        bucketLP
    );
}
@github-actions github-actions bot added the Medium A valid Medium severity issue label Oct 28, 2023
@ith-harvey ith-harvey added the Will Fix The sponsor confirmed this issue will be fixed label Nov 1, 2023
@sherlock-admin sherlock-admin changed the title Bumpy Punch Albatross - HPB may be incorrectly bankrupt due to use of unscaled value in _forgiveBadDebt 0xkaden - HPB may be incorrectly bankrupt due to use of unscaled value in _forgiveBadDebt Nov 7, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Nov 7, 2023
@ith-harvey
Copy link

Resolve here: ajna-finance/ajna-core#971

@dmitriia
Copy link
Collaborator

dmitriia commented Dec 8, 2023

Fix looks ok, remaining deposit quote funds are now scaled in _forgiveBadDebt() bankruptcy logic.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Medium A valid Medium severity issue Reward A payout will be made for this issue Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants