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.

Sign up for GitHub

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

Jump to bottom

Upgraded Q -> 2 from #815 [1701456736084] #887

Closed
c4-judge opened this issue Dec 1, 2023 · 3 comments
Closed

Upgraded Q -> 2 from #815 [1701456736084] #887

c4-judge opened this issue Dec 1, 2023 · 3 comments
Labels
downgraded by judge Judge downgraded the risk level of this issue duplicate-479 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax satisfactory satisfies C4 submission criteria; eligible for awards withdrawn by judge Special case: this finding was auto-generated by a judge and is now withdrawn; it can be ignored

Comments

@c4-judge
Copy link
Contributor

c4-judge commented Dec 1, 2023

Judge has assessed an item in Issue #815 as 2 risk. The relevant finding follows:

Chainlink price feed decimals not checked
The price value returned by a Chainlink price feed will have a different decimals value depending on the price feed used. While currently most ETH pairs use 18 decimals and USD pairs use 8 decimals (see the price feeds for LINK/ETH and LINK/USD for example), there is no guarantee that this will be the case for price feeds deployed in the future. If the decimals are not checked when querying a price feed, incorrect decimals may be assumed which can lead to significant accounting errors. Specifically, in LRTDepositPool#getRsETHAmountToMint, the decimals of getAssetPrice() is assumed to be exactly 18, otherwise the returned value could be far smaller than expected, leading to users being minted far fewer rsETH tokens than intended.

To access a price feeds decimals, simply call priceFeed.decimals().

https://github.com/code-423n4/2023-11-kelp/blob/main/src/oracles/ChainlinkPriceOracle.sol#L38
@c4-judge c4-judge added the 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value label Dec 1, 2023
c4-judge added a commit that referenced this issue Dec 1, 2023
@c4-judge
Madalad issue #887 (File generated from upgraded finding by judge fat…
863f944 
…herGoose1)
@c4-judge c4-judge closed this as completed Dec 1, 2023
@c4-judge
Copy link
Contributor Author

c4-judge commented Dec 1, 2023

fatherGoose1 marked the issue as duplicate of #479

@c4-judge c4-judge added duplicate-479 satisfactory satisfies C4 submission criteria; eligible for awards labels Dec 1, 2023
@c4-judge
Copy link
Contributor Author

c4-judge commented Dec 1, 2023

fatherGoose1 marked the issue as satisfactory

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax withdrawn by judge Special case: this finding was auto-generated by a judge and is now withdrawn; it can be ignored and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Dec 4, 2023
@c4-judge
Copy link
Contributor Author

c4-judge commented Dec 4, 2023

This auto-generated issue was withdrawn by fatherGoose1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
downgraded by judge Judge downgraded the risk level of this issue duplicate-479 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax satisfactory satisfies C4 submission criteria; eligible for awards withdrawn by judge Special case: this finding was auto-generated by a judge and is now withdrawn; it can be ignored
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@c4-judge