Skip to content
This repository has been archived by the owner on Jul 14, 2024. It is now read-only.

0xnirlin - Wrong number for blocks in a week #230

Closed
sherlock-admin2 opened this issue Jan 10, 2024 · 3 comments
Closed

0xnirlin - Wrong number for blocks in a week #230

sherlock-admin2 opened this issue Jan 10, 2024 · 3 comments
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout 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 Jan 10, 2024 •
edited
Loading

0xnirlin

high

Wrong number for blocks in a week

Summary

In a week number of blocks mined on eth currently are around 49500

Vulnerability Detail

Code set number of blocks in week as following diamondinit.sol

       // @note - this is wrong, number of blocks in a week is currently 49700
        ls.blockCountInAWeek = 45361;

Which leads to wrong calculations in whole project.

And also we know this number can change in future with hardforks.

Impact

wrong calculation

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/upgradeInitializers/DiamondInit.sol#L74

Tool used

smol cat

Recommendation

Set amount to right value .
@github-actions github-actions bot added the Excluded Excluded by the judge without consulting the protocol or the senior label Jan 14, 2024
@sherlock-admin2
Copy link
Contributor Author

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

It's protocol decision, dont have to be correct number though

1 similar comment
@sherlock-admin2
Copy link
Contributor Author

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

It's protocol decision, dont have to be correct number though

@rndquu rndquu mentioned this issue Jan 18, 2024
Closed
13 tasks
@nevillehuang
Copy link
Collaborator

Invalid, this is purely an estimate since block count fluctuates in mainnet. Additionally, it can be adjusted by admin anyways as seen here.

@sherlock-admin2 sherlock-admin2 changed the title Fit Tawny Crow - Wrong number for blocks in a week 0xnirlin - Wrong number for blocks in a week Jan 24, 2024
@sherlock-admin2 sherlock-admin2 added the Non-Reward This issue will not receive a payout label Jan 24, 2024
gitcoindev added a commit to gitcoindev/ubiquity-dollar that referenced this issue Feb 2, 2024
@gitcoindev
feat: implement BlocksInWeek script task
c6034d8 
The BlocksInWeek task provides a very close approximate of number of
blocks mined during one week.

Supported networks: mainnnet, sepolia.

Example usage:

npx tsx scripts/task/task.ts BlocksInWeek --network=mainnet
npx tsx scripts/task/task.ts BlocksInWeek --network=sepolia

Resolves: sherlock-audit/2023-12-ubiquity-judging#230
gitcoindev added a commit to gitcoindev/ubiquity-dollar that referenced this issue Feb 2, 2024
@gitcoindev
feat: implement BlocksInWeek script task
0e481f1 
The BlocksInWeek task provides a very close approximate of number of
blocks mined during one week.

Supported networks: mainnnet, sepolia.

Example usage:

npx tsx scripts/task/task.ts BlocksInWeek --network=mainnet
npx tsx scripts/task/task.ts BlocksInWeek --network=sepolia

Resolves: sherlock-audit/2023-12-ubiquity-judging#230
gitcoindev added a commit to gitcoindev/ubiquity-dollar that referenced this issue Feb 2, 2024
@gitcoindev
feat: update weekly block count to 49930
56207cb 
Set weekly block count to 49930 as measured in February 2024

npx tsx scripts/task/task.ts BlocksInWeek --network=mainnet
...
Calculating number of blocks in the last week...
Recent average block time: 12 seconds
Estimated blocks in a week best case 50400
Produced 49930 blocks, 470 worst than the best case

Resolves: sherlock-audit/2023-12-ubiquity-judging#230
@gitcoindev gitcoindev mentioned this issue Feb 2, 2024
Merged
molecula451 pushed a commit to ubiquity/ubiquity-dollar that referenced this issue Feb 2, 2024
@gitcoindev
Update block count in a week (#891)
aa55dad 
* feat: implement BlocksInWeek script task

The BlocksInWeek task provides a very close approximate of number of
blocks mined during one week.

Supported networks: mainnnet, sepolia.

Example usage:

npx tsx scripts/task/task.ts BlocksInWeek --network=mainnet
npx tsx scripts/task/task.ts BlocksInWeek --network=sepolia

Resolves: sherlock-audit/2023-12-ubiquity-judging#230

* feat: update weekly block count to 49930

Set weekly block count to 49930 as measured in February 2024

npx tsx scripts/task/task.ts BlocksInWeek --network=mainnet
...
Calculating number of blocks in the last week...
Recent average block time: 12 seconds
Estimated blocks in a week best case 50400
Produced 49930 blocks, 470 worst than the best case

Resolves: sherlock-audit/2023-12-ubiquity-judging#230

* feat: rename task function to funcBlocksInAWeek

As proposed during pull request review.
@sherlock-admin2 sherlock-admin2 added Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed labels Feb 17, 2024
molecula451 added a commit to ubiquity/ubiquity-dollar that referenced this issue Mar 1, 2024
@rndquu @molecula451 @gitcoindev
chore: Fix review (#907)
acc5800 
* fix: deprecate incentive on dollar token

* chore: update

* chore: update

* Fix set role admin (#880)

* feat: add setRoleAdmin to AccessControlFacet

The setRoleAdmin can be only accessed by the admin.

* feat: add setRoleAdmin to AccessControl interface

* test: add testSetRoleAdmin_ShouldSetAdminRoleForGivenRole test

* feat: update access control for setRoleAdmin

* test: fix ShouldSetAdminRoleForGivenRole and add test for revert

* feat: add getRedeemCollateralBalance() method

* fix: remove balanced reserves check (#883)

* fix: limit AMO minter borrow amount (#882)

* fix: limit AMO minter borrow amount

* test: assert free collateral amount

* fix: do not allow to mint dollar with zero collateral

Resolves: sherlock-audit/2023-12-ubiquity-judging#207

* test: add testMintDollar_ShouldRevert_IfZeroCollateralAvailable

* test: update comment in zero collateral mint test

* Update block count in a week (#891)

* feat: implement BlocksInWeek script task

The BlocksInWeek task provides a very close approximate of number of
blocks mined during one week.

Supported networks: mainnnet, sepolia.

Example usage:

npx tsx scripts/task/task.ts BlocksInWeek --network=mainnet
npx tsx scripts/task/task.ts BlocksInWeek --network=sepolia

Resolves: sherlock-audit/2023-12-ubiquity-judging#230

* feat: update weekly block count to 49930

Set weekly block count to 49930 as measured in February 2024

npx tsx scripts/task/task.ts BlocksInWeek --network=mainnet
...
Calculating number of blocks in the last week...
Recent average block time: 12 seconds
Estimated blocks in a week best case 50400
Produced 49930 blocks, 470 worst than the best case

Resolves: sherlock-audit/2023-12-ubiquity-judging#230

* feat: rename task function to funcBlocksInAWeek

As proposed during pull request review.

* feat: use CurveStableSwapMetaNG contract

* refactor: update migrations to use latest metapool

* refactor: deprecate IMetaPool

* refactor: remove MockTWAPOracleDollar3pool

* refactor: remove TWAPOracleDollar3poolFacet

* refactor: remove MockMetaPool

* refactor: remove LibTWAPOracle

* fix(dapp): remove TWAP oracle ABI import

* refactor(frontend): use ICurveStableSwapMetaNG for TWAP

* feat: check if collateral is enabled in collectRedemption (#894)

Also add a unit test that verifies the check.

Resolves: sherlock-audit/2023-12-ubiquity-judging#29

---------

Co-authored-by: molecula451 <web3samuraidev@gmail.com>
Co-authored-by: korrrba <88761781+gitcoindev@users.noreply.github.com>
Co-authored-by: Korrrba <gitcoiner@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Excluded Excluded by the judge without consulting the protocol or the senior Non-Reward This issue will not receive a payout Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@nevillehuang @sherlock-admin2