evmboi32 - Incorrect KeeperFee

[sherlock-admin]

evmboi32

medium

Incorrect KeeperFee

Summary

Keeper fee uses the wrong heartbeat when reading a chainlink oracle.

Vulnerability Detail

The KeeperFee.sol is the contract that calculates the keeperFee for the order execution. It uses the price of Ethereum to determine the execution cost based on a few different variables that are queried from the _gasPriceOracle.

The issue is that the _STALENESS_PERIOD is set as constant (wrong one). It is set as 1 day whereas the heartbeat for the ETH/USD chainlink feed is recommended to be 1200s on the BASE network.

This can be seen here in the official docs
https://docs.chain.link/data-feeds/price-feeds/addresses?network=base&page=1&search=eth%2F

This can lead to the use of a stale price for the keeper fee calculation.

Impact

The use of a stale price for keeperFee calculation can lead to a scenario where keepers execute orders based on a stale price. Because of that, the users could pay less for fees if the price goes up but this is not yet reflected on chain and keepers get less collateral for their service. It also goes the other way around if the price of Ethereum goes down and is not yet updated.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/misc/KeeperFee.sol#L29

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/misc/KeeperFee.sol#L95

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/misc/KeeperFee.sol#L109-L111

Tool used

Manual Review

Recommendation

Use the correct heartbeat of 1200s.

[sherlock-admin]

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

takarez commented:

invalid

[sherlock-admin]

The protocol team fixed this issue in PR/commit https://github.com/dhedge/flatcoin-v1/pull/271.

[nevillehuang]

I believe this is invalid based on comments here

[evmboi32]

Escalate

The linked comment is from the Ubiquity contest where an admin can change the heartbeat. Here in FlatMoney the heartbeat is a constant and cannot be changed. So in case of a stale price users could pay more than needed, or they pay to little.

[sherlock-admin2]

Escalate

The linked comment is from the Ubiquity contest where an admin can change the heartbeat. Here in FlatMoney the heartbeat is a constant and cannot be changed. So in case of a stale price users could pay more than needed, or they pay to little.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[Czar102]

Chainlink is trusted to provide up-to-date data. In this scenario, this is nothing more than a sanity check, and it's not required. For more information, see https://github.com/sherlock-audit/2023-12-flatmoney-judging/issues/245#issuecomment-1968811960.

Planning to reject the escalation and leave the issue as is.

[nevillehuang]

Agree with @Czar102, similar to #245, should remain as invalid.

[Czar102]

Result: Low Has duplicates

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• evmboi32: rejected