PaprController._reduceDebt: reverts if amount greater debt which can lead to DOS

[code423n4]

Lines of code

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L481
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L486-L489

Vulnerability details

Impact

There are two ways to reduce debt.

The first is by calling PaprController.reduceDebt (https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L148) which burns papr token from msg.sender directly.
The second is the PaprController.buyAndReduceDebt function (https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L208) which swaps the underlying token for papr token and then burns the papr token.

Both ways internally call PaprController._reduceDebt (https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L481).

The PaprController._reduceDebt function reverts if the amount parameter is bigger than the actual debt in the vault.

This is a problem for two reasons:

1. When a user wants to pay all his debt using the PaprController.buyAndReduceDebt function he must perform complex math in order to calculate the amount of input token that results in the exact amount of papr token

2. An attacker can front-run a transaction that repays all debt by paying back 1 Wei. The transaction to pay back the whole debt then reverts

The first issue is more a usability issue than a security issue.
However the second issue can be used to DOS the application. Especially if other protocols integrate with the papr protocol and are not flexible enough to pay back amounts that are smaller than the maximum.

Proof of Concept

PaprController._reduceDebt calls PaprController._reduceDebtWithoutBurn (https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L486-L489):

function _reduceDebtWithoutBurn(address account, ERC721 asset, uint256 amount) internal {
    _vaultInfo[account][asset].debt = uint200(_vaultInfo[account][asset].debt - amount);
    emit ReduceDebt(account, asset, amount);
}

This reverts if amount > _vaultInfo[account][asset].

Tools Used

VSCode

Recommended Mitigation Steps

If PaprController._reduceDebt is called with an amount > _vaultInfo[account][asset].debt, then amount should be set to _vaultInfo[account][asset].debt.

So PaprController._reduceDebt should look like this:

function _reduceDebt(address account, ERC721 asset, address burnFrom, uint256 amount) internal {
    if (amount > _vaultInfo[account][asset].debt) {
        amount = _vaultInfo[account][asset].debt;
    }
    _reduceDebtWithoutBurn(account, asset, amount);
    PaprToken(address(papr)).burn(burnFrom, amount);
}

[c4-judge]

trust1995 marked the issue as duplicate of #92

[c4-judge]

trust1995 marked the issue as satisfactory