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

Madalad - Missing check if Chainlink sequencer is down #29

Closed
sherlock-admin opened this issue Aug 15, 2023 · 1 comment
Closed

Madalad - Missing check if Chainlink sequencer is down #29

sherlock-admin opened this issue Aug 15, 2023 · 1 comment
Labels
Non-Reward This issue will not receive a payout

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Aug 15, 2023

Madalad

medium

Missing check if Chainlink sequencer is down

Summary

Call to Chainlink price feed may return inaccurate value on L2 if the sequencer is unavailable.

Vulnerability Detail

When utilizing Chainlink in L2 chains like Arbitrum or Optimism, it's important
to ensure that the prices provided are not falsely perceived as fresh, even
when the sequencer is down. This vulnerability could potentially be exploited
by malicious actors to gain an unfair advantage.

See Chainlink's docs
for more information.

Impact

Valuing assets incorrectly can lead to unexpected behaviour such as undercollateralization or unfair liquidations.

Code Snippet

File: root\contracts\attribute\Kept.sol

59:     /// @notice Returns the price of ETH in terms of the keeper token
60:     /// @return The price of ETH in terms of the keeper token
61:     function _etherPrice() private view returns (UFixed18) {
62:         (, int256 answer, , ,) = ethTokenOracleFeed().latestRoundData();
63:         return UFixed18Lib.from(Fixed18Lib.ratio(answer, 1e8)); // chainlink eth-usd feed uses 8 decimals
64:     }

https://github.com/sherlock-audit/2023-07-perennial/blob/main/root/contracts/attribute/Kept.sol#L62

Tool used

Manual Review

Recommendation

Implement a sequencer uptime check if the contract is deployed to an L2, as shown here.

Duplicate of #146

@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Aug 18, 2023
@sherlock-admin
Copy link
Contributor Author

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

141345 commented:

d

n33k commented:

unhandled stale price returned from latestRoundData()

@sherlock-admin sherlock-admin changed the title Crazy Rainbow Viper - Missing check if Chainlink sequencer is down Madalad - Missing check if Chainlink sequencer is down Aug 23, 2023
@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Non-Reward This issue will not receive a payout
Projects
None yet
Development

No branches or pull requests

1 participant