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

0x73696d616f - Rebasing tokens are not supported contrary to the readme and will lead to loss of funds #235

Open
sherlock-admin2 opened this issue Sep 10, 2024 · 1 comment
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A Medium severity issue. Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Sep 10, 2024

0x73696d616f

Medium

Rebasing tokens are not supported contrary to the readme and will lead to loss of funds

Summary

The readme states that rebasing tokens are supported

Rebasing tokens are supported with exchange rate mechanism

However, only non rebasing tokens such as the wrapped version wsteth are supposed. If stETH is used, it will accrue value in the Psm and Vault (technically they are the same contract) which will be left untracked as Ra and Pa deposits are tracked in state variables.

Root Cause

The code does not handle rebasing tokens even though the readme says it does. The exchange rate mechanism only supports non rebasing tokens such as wsteth.

Internal pre-conditions

None.

External pre-conditions

None.

Attack Path

Admin creates steth pairs using it as Ra or Pa, whose value will grow in the protocol but left untracked as the quantites are tracked with state variables.

Impact

Stuck yield accruel in the Vault/Psm contracts.

PoC

State.sol tracks the balances:

struct Balances {
    PsmRedemptionAssetManager ra;
    uint256 dsBalance;
    uint256 ctBalance;
    uint256 paBalance;
}

Mitigation

Don't set rebasing tokens are Ra or Pa or implement a way to sync the balances.

@ziankork
Copy link
Collaborator

this is intended as there's no easy way to sync balance between psm and vault since when we sync we have to somehow calculate the virtual reserves of each PSM and Vault from the total shares that the contract holds. so won't fix

@sherlock-admin3 sherlock-admin3 added Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed labels Sep 25, 2024
@sherlock-admin3 sherlock-admin3 changed the title Blurry Blush Mouse - Rebasing tokens are not supported contrary to the readme and will lead to loss of funds 0x73696d616f - Rebasing tokens are not supported contrary to the readme and will lead to loss of funds Sep 25, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Sep 25, 2024
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 Medium A Medium severity issue. Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

3 participants