Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grieving attack by failing user's transactions #92

Open
code423n4 opened this issue Dec 20, 2022 · 5 comments
Open

Grieving attack by failing user's transactions #92

code423n4 opened this issue Dec 20, 2022 · 5 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden M-03 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

code423n4 commented Dec 20, 2022

Lines of code

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

Vulnerability details

Impact

An attacker can apply grieving attack by preventing users from interacting with some of the protocol functions. In other words whenever a user is going to reduce his debt, or buy and reduce his debt in one tx, it can be failed by the attacker.

Proof of Concept

In the following scenario, I am explaining how it is possible to fail user's transaction to reduce their debt fully. Failing other transaction (buy and reduce the debt in one tx) can be done similarly.

  • Suppose Alice (an honest user) has debt of 1000 PaprToken and she intends to repay her debt fully:
  • So, she calls the function reduceDebt with the following parameters:
    • account: Alice's address
    • asset: The NFT which was used as collateral
    • amount: 1000 * 10**18 (decimal of PaprToken is 18).
function reduceDebt(address account, ERC721 asset, uint256 amount) external override {
        _reduceDebt({account: account, asset: asset, burnFrom: msg.sender, amount: amount});
    }

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

  • Bob (a malicious user who owns a small amount of PaprToken) notices Alice's transaction in the Mempool. So, Bob applies front-run attack and calls the function reduceDebt with the following parameters:
    • account: Alice's address
    • asset: The NFT which was used as collateral
    • amount: 1
  • By doing so, Bob repays only 1 PaprToken on behalf of Alice, so Alice's debt becomes 1000 * 10**18 - 1.
function _reduceDebt(address account, ERC721 asset, address burnFrom, uint256 amount) internal {
        _reduceDebtWithoutBurn(account, asset, amount);
        PaprToken(address(papr)).burn(burnFrom, amount);
    }

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

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

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

  • Then, when Alice's transaction is going to be executed, it fails because of Underflow Error. Since Alice's debt is 1000 * 10**18 - 1 while Alice's transaction was going to repay 1000 * 10**18.
_vaultInfo[account][asset].debt = uint200(_vaultInfo[account][asset].debt - amount);

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

  • Bob only pays a very small value of 1 PaprToken (consider that the decimal is 18) to apply this grieving attack.
  • Bob can repeat this attack for Alice, if Alice is going to call this function again with correct parameter.

In summary, Bob could prevent the user from paying her debt fully by just repaying a very small amount of the user's debt in advance and as a result causing underflow error. Bob can apply this attack for all other users who are going to repay their debt fully. Please note that if a user is going to repay her debt partially, the attack can be expensive and not financially reasonable, but in case of full repayment of debt, it is very cheap to apply this grieving attack.

This attack can be applied on the transactions that are going to interact with the function _reduceDebt. The transactions interacting with this specific function are:

It means that the attacker can prevent users from calling the functions above.

Tools Used

Recommended Mitigation Steps

The following condition should be added to the function _reduceDebtWithoutBurn:

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

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

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 20, 2022
code423n4 added a commit that referenced this issue Dec 20, 2022
@code423n4 code423n4 changed the title Grieving attack by preventing users from paying their full debt Grieving attack by failing user's transactions Dec 21, 2022
@trust1995
Copy link

Right on the edge between M and L+, leaning towards M.

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Dec 25, 2022
@c4-judge
Copy link
Contributor

trust1995 marked the issue as satisfactory

@c4-judge
Copy link
Contributor

trust1995 marked the issue as primary issue

@c4-judge c4-judge added the primary issue Highest quality submission among a set of duplicates label Dec 25, 2022
@c4-sponsor
Copy link

wilsoncusack marked the issue as sponsor confirmed

@c4-sponsor c4-sponsor added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Jan 3, 2023
@c4-judge
Copy link
Contributor

c4-judge commented Jan 4, 2023

trust1995 marked the issue as selected for report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden M-03 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

5 participants