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

Seed Gauge System Codehawks Remediations #819

Merged
merged 149 commits into from
Apr 10, 2024

Conversation

Brean0
Copy link
Contributor

@Brean0 Brean0 commented Apr 6, 2024

Seed Gauge System Codehawks Changes

Contest link: https://www.codehawks.com/report/clsxlpte900074r5et7x6kh96

Codehawks Remediations

Other Contract Changes

  • Increase the maximum Beans that can be rewarded during a gm call in the first available block to 250 beans. (fc905c7)
  • Updated enrootDeposits to correctly emit ERC-1155 events. (7b98fe1) (see. EnrootFacet, TokenSilo, LibSilo)
  • Fixed a bug where Beanstalk returned the USD/BEAN price instead of BEAN/USD when checking for excessive price. (f96a7ab)

Misc. changes

Accepted Risks from Codehawks Findings

M-06. [M] DOS in LibChainlinkOracle when not considering phaseId.

This occurs when the Chainlink oracle updates their phaseAggergator. This has occurred 6 times in the lifetime of the ETH/USD oracle. When a change occurs, Beanstalk will not be able to determine a TWAP for the season, causing a 1 Season DoS. In order to determine the next roundId to query when a phase a phaseAggergator change occurs, Beanstalk needs to be able to call latestRound() on the phase aggregator, which contains access control and thus can only be called by specific contracts. This is an acceptable edge case due to the infrequency of this occurring.

L-01. LibEthUsdOracle returning wrong price on minAnswer, impacting fertilizer minting

The minAnswer of the ETH/USD oracle is 1, which given the decimals precision of 8, means that Ether would need to be lower than 1-e8 dollars in order to effect the protocol. This seems unlikely and unworthy of implementing logic to account for the edge case.

L-02. No validation of total supply of unripe beans & Lp in percentBeansRecapped & percentLPRecapped

In the case that all Unripe assets are Chopped/Converted, users would not be able to Convert between Unripe assets. Given that this would only occur when all assets are chopped, it seems preferable to accept this edge case instead of introducing new logic that may be exploitable.

@Brean0 Brean0 merged commit bd25667 into bip39-seedGauge Apr 10, 2024
0 of 7 checks passed
@Brean0 Brean0 deleted the bip39-seedGauge-remediations branch April 10, 2024 15:46
@Brean0 Brean0 restored the bip39-seedGauge-remediations branch April 11, 2024 16:51
@hellofromguy hellofromguy changed the title Bip39 seed gauge remediations Seed Gauge System Codehawks Remediations Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants