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

Obsidian - The totalAssets() function is not ERC 4626 compliant #560

Closed
sherlock-admin2 opened this issue Aug 24, 2024 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label 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 Aug 24, 2024

Obsidian

Medium

The totalAssets() function is not ERC 4626 compliant

Summary

ERC-4626 clearly states the following about the totalAssets() function:

MUST be inclusive of any fees that are charged against assets in the Vault.

The issue is that the protocol's implementation of the function does not account for the fact that some of the assets are the accumulated interest fee.

The protocol accumulates the interest fee by minting shares to the feeRecipient() every time the accrue function is called.

The totalAssets() function should subtract the accumulated interest fee from the currently calculated amount of total assets to return a value that is including the fees charged on interest.

Root Cause

totalAssets() does not subtract the accumulated interest fees

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

SuperPool is not ERC-4626 compliant, this can pose issues with external integrations that expect strict compliance

PoC

No response

Mitigation

Implement a variable to track the fee amount, subtract it from totalAssets()

This may require redesigning functions that implement totalAssets()

Duplicate of #110

@github-actions github-actions bot added the Medium A Medium severity issue. label Sep 5, 2024
@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 10, 2024
@z3s z3s added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Sep 15, 2024
@z3s z3s closed this as completed Sep 15, 2024
@sherlock-admin4 sherlock-admin4 changed the title Glamorous Blush Gecko - The totalAssets() function is not ERC 4626 compliant Obsidian - The totalAssets() function is not ERC 4626 compliant Sep 15, 2024
@sherlock-admin4 sherlock-admin4 added the Reward A payout will be made for this issue label Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label 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

4 participants