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

sakshamguruji - Incorrect redeemAmount Is Accounted Due To Not Accounting For The Exchange Rate #119

Open
sherlock-admin2 opened this issue Sep 10, 2024 · 2 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A High severity issue. Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Sep 10, 2024

sakshamguruji

High

Incorrect redeemAmount Is Accounted Due To Not Accounting For The Exchange Rate

Summary

When Liquidating LP , DS and CT are paired , then that amount is used to redeem RA . But the accounting for RA has been done incorrectly since it does not account for exchange rate.

Vulnerability Detail

1.) Inside Liquidate LP we empty out the DS reserve and pair it up with the CT amount returned from the AMM ->

https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultLib.sol#L376

This is the amount of CtDs being redeemed.

2.) This same amount has been accounted for the increment in RA ->

https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultLib.sol#L390

But this is incorrect , this is because redeemAmount is an amount in Ct/Ds not in RA , to make it into RA we need to apply the exchange rate over it(exchange rate is how many Redemption Assets you need to deposit to receive 1 Cover Token + 1 Depeg Swap and how many Redemption Assets you receive when redeeming 1 Pegged Asset + 1 Depeg Swap , read more in the Dealing with non-rebasing Pegged Assets section -> https://corkfi.notion.site/Cork-Protocol-Litepaper-f21a57d5c19d48209dfa0f0c2ab776c4).

3.) Therefore incorrect RA amount has been accounted and incorrect amount of RA would be reserved ->

https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultLib.sol#L392

meaning , incorrect amount of RA attributed to be withdrawn/redeemed.

4.) This inconsistency is found at multiple places , and instead of making separate reports im listing them here ->

a.) https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/PsmLib.sol#L122

The amount here is in RA and we are issuing DS/CT with it.

b.) https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/core/flash-swaps/FlashSwapRouter.sol#L368

dsAttributed is in DS and we are depositing RA.

c.) https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultLib.sol#L331

redeemAmount is in Ct/Ds here

Impact

Code Snippet

https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/VaultLib.sol#L390

Tool used

Manual Review

Recommendation

Account for the exchange rate correctly.

@github-actions github-actions bot added Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A High severity issue. labels Sep 14, 2024
This was referenced Sep 14, 2024
@sherlock-admin3 sherlock-admin3 added the Sponsor Confirmed The sponsor acknowledged this issue is valid label Sep 23, 2024
@ziankork
Copy link
Collaborator

This is valid, will fix

@sherlock-admin3 sherlock-admin3 added the Will Fix The sponsor confirmed this issue will be fixed label Sep 23, 2024
@sherlock-admin3 sherlock-admin3 changed the title Jumpy Lime Oyster - Incorrect redeemAmount Is Accounted Due To Not Accounting For The Exchange Rate sakshamguruji - Incorrect redeemAmount Is Accounted Due To Not Accounting For The Exchange Rate Sep 25, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Sep 25, 2024
@sherlock-admin2
Copy link
Contributor Author

The protocol team fixed this issue in the following PRs/commits:
Cork-Technology/Depeg-swap#81

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A High severity issue. Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

3 participants