code-423n4 / 2023-10-badger-findings

1 stars 1 forks source link

`PriceFeed.TIMEOUT_ETH_BTC_FEED` and `PriceFeed.TIMEOUT_STETH_ETH_FEED` are set too large #191

Closed c4-submissions closed 11 months ago

c4-submissions commented 12 months ago

Lines of code

https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/PriceFeed.sol#L32-L33

Vulnerability details

Impact

PriceFeed.TIMEOUT_ETH_BTC_FEED and PriceFeed.TIMEOUT_STETH_ETH_FEED are used as:

Maximum time period allowed since Chainlink's latest round data timestamp, beyond which Chainlink is considered frozen.

If those two constants are set too large, the system will detect Chainlink not working later than expect, which might cause the system uses stale price.

Proof of Concept

Quoting the code

    // Maximum time period allowed since Chainlink's latest round data timestamp, beyond which Chainlink is considered frozen.
    uint256 public constant TIMEOUT_ETH_BTC_FEED = 4800; // 1 hours & 20min: 60 * 80
    uint256 public constant TIMEOUT_STETH_ETH_FEED = 90000; // 25 hours: 60 * 60 * 25

Then I checked the aggregators for ETH_BTC_CL_FEED and STETH_ETH_CL_FEED.According etherscan's dashboard, Transmit method tx is used to update the price. So I download the Transmit type of tx from etherscan as csv file(for ETH_BTC, I download 03/01/2023 ~ 10/01/2023 because of 5000 records limit, and for STETH_ETH, I download all the data), and then and with the help of following python script, The MAX TIME GAP for TIMEOUT_ETH_BTC_FEED should be less than 3 mins, and The MAX TIME GAP for TIMEOUT_STETH_ETH_FEED should be less than 5 mins

So TIMEOUT_ETH_BTC_FEED = 4800 and TIMEOUT_STETH_ETH_FEED = 90000 are too large

import functools
import sys
import re

old_val = 0
new_val = 0
max_val = 0 # max update timestap gap
cnt     = 0

with open(sys.argv[1]) as f:
    f.readline()  # skip the first line

    for line in f:
        try:
            time_str = line.strip().split(",")[2] # get UnixTimestamp colunm data
            time_str = re.sub("\"", "", time_str) # remove `"` in UnixTimestamp
            value = int(time_str)
            if 0 == cnt:
                new_val = value
                cnt = 1;
            else:
                diff = (value - new_val) / (60)
                if diff > max_val:
                    max_val = diff
                old_val = new_val
                new_val = value
                print(old_val, new_val, diff)
        except ValueError:
            print("Error converting to int:")
print(max_val)

Tools Used

VIM

Recommended Mitigation Steps

change PriceFeed.TIMEOUT_ETH_BTC_FEED and PriceFeed.TIMEOUT_STETH_ETH_FEED to a smaller value

Assessed type

Other

c4-pre-sort commented 12 months ago

bytes032 marked the issue as insufficient quality report

bytes032 commented 12 months ago

OOS

jhsagd76 commented 11 months ago

https://badger.com/images/uploads/ebtc-security-review-spearbit.pdf

5.3.16 PriceFeed is not considering that the "Trigger parameters" of the Chainlink oracle could change

c4-judge commented 11 months ago

jhsagd76 marked the issue as unsatisfactory: Out of scope