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

Discount mechanism can be broken by using 0 as amount/value #102

Open
hats-bug-reporter bot opened this issue Sep 15, 2024 · 1 comment
Open

Discount mechanism can be broken by using 0 as amount/value #102

hats-bug-reporter bot opened this issue Sep 15, 2024 · 1 comment
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

Github username: --
Twitter username: --
Submission hash (on-chain): 0x1e8efe52cc159b68e01c7ad30ecadaccc82dfa61d79296a1a55043b1d9c88433
Severity: high

Description:
Description
I'll take Hub.migrate as an example,

  1. In Hub.migrate, the function doesn't check if input parameter _amounts contains elements has 0 as input.
  2. then the function will call _mintAndUpdateTotalSupply
  3. in Circles._mintAndUpdateTotalSupply, the function also dosn't check if _value is larger than 0, and then calls _mint
  4. in ERC1155._mint, the function doesn't check if value is larger than 0 and calls ERC1155._updateWithAcceptanceCheck, also the function doesn't check values
  5. And finally in ERC1155._update, the funciton also doesn't check if values is zero
  6. Then back to Circles._mintAndUpdateTotalSupply function, the newTotalSupply is calculated based on _calculateDiscountedBalance, and today - totalSupplyBalance.lastUpdatedDay is used
  7. According to Demurrage._calculateDiscountedBalance, if _daysDifference == 0, the balance will not be discounted
  8. And finally totalSupplyBalance.lastUpdatedDay is updated in Circles.sol#L161

Attack Scenario\

Please note that the issue exists in multiple functions in the protocol, such as ERC1155.safeTransferFrom and ERC1155.safeBatchTransferFrom and other functions.

For safeTransferFrom and safeBatchTransferFrom, the user can call this function with 0 every 23 hours to avoid be discounted.
Attachments

  1. Proof of Concept (PoC) File
  1. Revised Code File (Optional)
@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Sep 15, 2024
@benjaminbollen
Copy link

  • if called with zero value, it will simply and correctly update the total supply with demurrage. So not an issue.
  • specifically Zero-Amount Vulnerability in migrate Function Allows Unauthorized Migration #50 already pointed out the unintended design of the migrate function in Migrate.sol not checking for zero value; while this is not a security issue, it wasn't what we had considered, so we'll add a check and the issue is already covered

@benjaminbollen benjaminbollen added the invalid This doesn't seem right label Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant