code-423n4 / 2024-04-dyad-findings

8 stars 6 forks source link

`redeemDyad` and `burnDyad` function calls can be griefed by other users when repaying full amount #74

Closed c4-bot-3 closed 5 months ago

c4-bot-3 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/44becc2f09c3a75bd548d5ec756a8e88a345e826/src/core/VaultManagerV2.sol#L172-L203

Vulnerability details

Impact

The function burnDyad is open for all, meaning that any user can remove dyad debt from any other user's account. The function only has the isValidDNft modifier, which checks if the nft exists, not the ownership.

The issue is that if a user wants to repay the complete debt of their own account, by either calling burnDyad or redeemDyad, another user can call burnDyad on their account and burn 1 wei frontrunning the transaction. This will cause the owner's transaction to revert, since the system will be trying to burn more debt than the user accrued.

function burnDyad(
    uint id,
    uint amount
  )
    external
      isValidDNft(id)
  {
    dyad.burn(id, msg.sender, amount);
    emit BurnDyad(id, amount, msg.sender);
  }

The dyad contract ensures that users cannot burn more tokens than they minted via an underflow protection.

_burn(from, amount);
mintedDyad[msg.sender][id] -= amount;

So users will be griefed and will be unable to pay off their full debt. This is a griefing attack and is thus a medium severity issue.

Proof of Concept

Assume Alice has a debt of 100 dyad. She calls burnDyad with 100. Bob frontruns this transaction and pays off 1 wei of Alice's debt. Alice's transaction now fails, due to her trying to repay more debt than she has.

Tools Used

Manual review

Recommended Mitigation Steps

Add a code section that truncates the amount to pay in case a larger amount is passed in for both burnDyad and redeemDyad.

if(amount > dyad.mintedDyad(address(this), id)) amount = dyad.mintedDyad(address(this), id);

Assessed type

DoS

c4-pre-sort commented 5 months ago

JustDravee marked the issue as primary issue

c4-pre-sort commented 5 months ago

JustDravee marked the issue as sufficient quality report

shafu0x commented 5 months ago

yeah, burn dyad should only be done by the owner.

c4-judge commented 5 months ago

koolexcrypto marked the issue as satisfactory

c4-judge commented 5 months ago

koolexcrypto marked the issue as selected for report

c4-judge commented 5 months ago

koolexcrypto marked the issue as not selected for report

c4-judge commented 5 months ago

koolexcrypto removed the grade

c4-judge commented 5 months ago

koolexcrypto marked the issue as duplicate of #992

c4-judge commented 5 months ago

koolexcrypto marked the issue as satisfactory

c4-judge commented 4 months ago

koolexcrypto marked the issue as duplicate of #100