Add Pyth unchecked confidence detector

pull/2581/head
Simone 3 weeks ago
parent 0811cd68e0
commit f25344c6a2
  1. 2
      slither/detectors/all_detectors.py
  2. 79
      slither/detectors/statements/pyth_unchecked.py
  3. 50
      slither/detectors/statements/pyth_unchecked_confidence.py
  4. 3
      tests/e2e/detectors/snapshots/detectors__detector_PythUncheckedConfidence_0_8_20_pyth_unchecked_confidence_sol__0.txt
  5. 193
      tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol
  6. BIN
      tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol-0.8.20.zip
  7. 10
      tests/e2e/detectors/test_detectors.py

@ -97,5 +97,7 @@ from .operations.incorrect_exp import IncorrectOperatorExponentiation
from .statements.tautological_compare import TautologicalCompare from .statements.tautological_compare import TautologicalCompare
from .statements.return_bomb import ReturnBomb from .statements.return_bomb import ReturnBomb
from .functions.out_of_order_retryable import OutOfOrderRetryable from .functions.out_of_order_retryable import OutOfOrderRetryable
from .statements.pyth_unchecked_confidence import PythUncheckedConfidence
from .statements.pyth_unchecked_publishtime import PythUncheckedPublishTime
# from .statements.unused_import import UnusedImport # from .statements.unused_import import UnusedImport

@ -0,0 +1,79 @@
from typing import List
from slither.detectors.abstract_detector import (
AbstractDetector,
DETECTOR_INFO,
)
from slither.utils.output import Output
from slither.slithir.operations import Member, Binary, Assignment
class PythUnchecked(AbstractDetector):
"""
Documentation: This detector finds deprecated Pyth function calls
"""
# To be overriden in the derived class
PYTH_FUNCTIONS = []
PYTH_FIELD = ""
# pylint: disable=too-many-nested-blocks
def _detect(self) -> List[Output]:
results: List[Output] = []
for contract in self.compilation_unit.contracts_derived:
for target_contract, ir in contract.all_high_level_calls:
if target_contract.name == "IPyth" and ir.function_name in self.PYTH_FUNCTIONS:
# We know for sure the second IR in the node is an Assignment operation of the TMP variable. Example:
# Expression: price = pyth.getEmaPriceNoOlderThan(id,age)
# IRs:
# TMP_0(PythStructs.Price) = HIGH_LEVEL_CALL, dest:pyth(IPyth), function:getEmaPriceNoOlderThan, arguments:['id', 'age']
# price(PythStructs.Price) := TMP_0(PythStructs.Price)
assert isinstance(ir.node.irs[1], Assignment)
return_variable = ir.node.irs[1].lvalue
checked = False
possible_unchecked_variable_ir = None
nodes = ir.node.sons
visited = set()
while nodes:
if checked:
break
next_node = nodes[0]
nodes = nodes[1:]
for node_ir in next_node.all_slithir_operations():
# We are accessing the unchecked_var field of the returned Price struct
if (
isinstance(node_ir, Member)
and node_ir.variable_left == return_variable
and node_ir.variable_right.name == self.PYTH_FIELD
):
possible_unchecked_variable_ir = node_ir.lvalue
# We assume that if unchecked_var happens to be inside a binary operation is checked
if (
isinstance(node_ir, Binary)
and possible_unchecked_variable_ir is not None
and possible_unchecked_variable_ir in node_ir.read
):
checked = True
break
if next_node not in visited:
visited.add(next_node)
for son in next_node.sons:
if son not in visited:
nodes.append(son)
if not checked:
info: DETECTOR_INFO = [
f"Pyth price {self.PYTH_FIELD} field is not checked in ",
ir.node.function,
"\n\t- ",
ir.node,
"\n",
]
res = self.generate_result(info)
results.append(res)
return results

@ -0,0 +1,50 @@
from slither.detectors.abstract_detector import DetectorClassification
from slither.detectors.statements.pyth_unchecked import PythUnchecked
class PythUncheckedConfidence(PythUnchecked):
"""
Documentation: This detector finds when the confidence level of a Pyth price is not checked
"""
ARGUMENT = "pyth-unchecked-confidence"
HELP = "Detect when the confidence level of a Pyth price is not checked"
IMPACT = DetectorClassification.MEDIUM
CONFIDENCE = DetectorClassification.HIGH
WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#pyth-unchecked-confidence"
WIKI_TITLE = "Pyth unchecked confidence level"
WIKI_DESCRIPTION = "Detect when the confidence level of a Pyth price is not checked"
WIKI_RECOMMENDATION = "Check the confidence level of a Pyth price. Visit https://docs.pyth.network/price-feeds/best-practices#confidence-intervals for more information."
WIKI_EXPLOIT_SCENARIO = """
```solidity
import "@pythnetwork/pyth-sdk-solidity/IPyth.sol";
import "@pythnetwork/pyth-sdk-solidity/PythStructs.sol";
contract C {
IPyth pyth;
constructor(IPyth _pyth) {
pyth = _pyth;
}
function bad(bytes32 id, uint256 age) public {
PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age);
// Use price
}
}
```
The function `A` uses the price without checking its confidence level.
"""
PYTH_FUNCTIONS = [
"getEmaPrice",
"getEmaPriceNoOlderThan",
"getEmaPriceUnsafe",
"getPrice",
"getPriceNoOlderThan",
"getPriceUnsafe",
]
PYTH_FIELD = "conf"

@ -0,0 +1,3 @@
Pyth price conf field is not checked in C.bad(bytes32,uint256) (tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol#171-175)
- price = pyth.getEmaPriceNoOlderThan(id,age) (tests/e2e/detectors/test_data/pyth-unchecked-confidence/0.8.20/pyth_unchecked_confidence.sol#172)

@ -0,0 +1,193 @@
contract PythStructs {
// A price with a degree of uncertainty, represented as a price +- a confidence interval.
//
// The confidence interval roughly corresponds to the standard error of a normal distribution.
// Both the price and confidence are stored in a fixed-point numeric representation,
// `x * (10^expo)`, where `expo` is the exponent.
//
// Please refer to the documentation at https://docs.pyth.network/consumers/best-practices for how
// to how this price safely.
struct Price {
// Price
int64 price;
// Confidence interval around the price
uint64 conf;
// Price exponent
int32 expo;
// Unix timestamp describing when the price was published
uint publishTime;
}
// PriceFeed represents a current aggregate price from pyth publisher feeds.
struct PriceFeed {
// The price ID.
bytes32 id;
// Latest available price
Price price;
// Latest available exponentially-weighted moving average price
Price emaPrice;
}
}
interface IPyth {
/// @notice Returns the period (in seconds) that a price feed is considered valid since its publish time
function getValidTimePeriod() external view returns (uint validTimePeriod);
/// @notice Returns the price and confidence interval.
/// @dev Reverts if the price has not been updated within the last `getValidTimePeriod()` seconds.
/// @param id The Pyth Price Feed ID of which to fetch the price and confidence interval.
/// @return price - please read the documentation of PythStructs.Price to understand how to use this safely.
function getPrice(
bytes32 id
) external view returns (PythStructs.Price memory price);
/// @notice Returns the exponentially-weighted moving average price and confidence interval.
/// @dev Reverts if the EMA price is not available.
/// @param id The Pyth Price Feed ID of which to fetch the EMA price and confidence interval.
/// @return price - please read the documentation of PythStructs.Price to understand how to use this safely.
function getEmaPrice(
bytes32 id
) external view returns (PythStructs.Price memory price);
/// @notice Returns the price of a price feed without any sanity checks.
/// @dev This function returns the most recent price update in this contract without any recency checks.
/// This function is unsafe as the returned price update may be arbitrarily far in the past.
///
/// Users of this function should check the `publishTime` in the price to ensure that the returned price is
/// sufficiently recent for their application. If you are considering using this function, it may be
/// safer / easier to use either `getPrice` or `getPriceNoOlderThan`.
/// @return price - please read the documentation of PythStructs.Price to understand how to use this safely.
function getPriceUnsafe(
bytes32 id
) external view returns (PythStructs.Price memory price);
/// @notice Returns the price that is no older than `age` seconds of the current time.
/// @dev This function is a sanity-checked version of `getPriceUnsafe` which is useful in
/// applications that require a sufficiently-recent price. Reverts if the price wasn't updated sufficiently
/// recently.
/// @return price - please read the documentation of PythStructs.Price to understand how to use this safely.
function getPriceNoOlderThan(
bytes32 id,
uint age
) external view returns (PythStructs.Price memory price);
/// @notice Returns the exponentially-weighted moving average price of a price feed without any sanity checks.
/// @dev This function returns the same price as `getEmaPrice` in the case where the price is available.
/// However, if the price is not recent this function returns the latest available price.
///
/// The returned price can be from arbitrarily far in the past; this function makes no guarantees that
/// the returned price is recent or useful for any particular application.
///
/// Users of this function should check the `publishTime` in the price to ensure that the returned price is
/// sufficiently recent for their application. If you are considering using this function, it may be
/// safer / easier to use either `getEmaPrice` or `getEmaPriceNoOlderThan`.
/// @return price - please read the documentation of PythStructs.Price to understand how to use this safely.
function getEmaPriceUnsafe(
bytes32 id
) external view returns (PythStructs.Price memory price);
/// @notice Returns the exponentially-weighted moving average price that is no older than `age` seconds
/// of the current time.
/// @dev This function is a sanity-checked version of `getEmaPriceUnsafe` which is useful in
/// applications that require a sufficiently-recent price. Reverts if the price wasn't updated sufficiently
/// recently.
/// @return price - please read the documentation of PythStructs.Price to understand how to use this safely.
function getEmaPriceNoOlderThan(
bytes32 id,
uint age
) external view returns (PythStructs.Price memory price);
/// @notice Update price feeds with given update messages.
/// This method requires the caller to pay a fee in wei; the required fee can be computed by calling
/// `getUpdateFee` with the length of the `updateData` array.
/// Prices will be updated if they are more recent than the current stored prices.
/// The call will succeed even if the update is not the most recent.
/// @dev Reverts if the transferred fee is not sufficient or the updateData is invalid.
/// @param updateData Array of price update data.
function updatePriceFeeds(bytes[] calldata updateData) external payable;
/// @notice Wrapper around updatePriceFeeds that rejects fast if a price update is not necessary. A price update is
/// necessary if the current on-chain publishTime is older than the given publishTime. It relies solely on the
/// given `publishTimes` for the price feeds and does not read the actual price update publish time within `updateData`.
///
/// This method requires the caller to pay a fee in wei; the required fee can be computed by calling
/// `getUpdateFee` with the length of the `updateData` array.
///
/// `priceIds` and `publishTimes` are two arrays with the same size that correspond to senders known publishTime
/// of each priceId when calling this method. If all of price feeds within `priceIds` have updated and have
/// a newer or equal publish time than the given publish time, it will reject the transaction to save gas.
/// Otherwise, it calls updatePriceFeeds method to update the prices.
///
/// @dev Reverts if update is not needed or the transferred fee is not sufficient or the updateData is invalid.
/// @param updateData Array of price update data.
/// @param priceIds Array of price ids.
/// @param publishTimes Array of publishTimes. `publishTimes[i]` corresponds to known `publishTime` of `priceIds[i]`
function updatePriceFeedsIfNecessary(
bytes[] calldata updateData,
bytes32[] calldata priceIds,
uint64[] calldata publishTimes
) external payable;
/// @notice Returns the required fee to update an array of price updates.
/// @param updateData Array of price update data.
/// @return feeAmount The required fee in Wei.
function getUpdateFee(
bytes[] calldata updateData
) external view returns (uint feeAmount);
/// @notice Parse `updateData` and return price feeds of the given `priceIds` if they are all published
/// within `minPublishTime` and `maxPublishTime`.
///
/// You can use this method if you want to use a Pyth price at a fixed time and not the most recent price;
/// otherwise, please consider using `updatePriceFeeds`. This method does not store the price updates on-chain.
///
/// This method requires the caller to pay a fee in wei; the required fee can be computed by calling
/// `getUpdateFee` with the length of the `updateData` array.
///
///
/// @dev Reverts if the transferred fee is not sufficient or the updateData is invalid or there is
/// no update for any of the given `priceIds` within the given time range.
/// @param updateData Array of price update data.
/// @param priceIds Array of price ids.
/// @param minPublishTime minimum acceptable publishTime for the given `priceIds`.
/// @param maxPublishTime maximum acceptable publishTime for the given `priceIds`.
/// @return priceFeeds Array of the price feeds corresponding to the given `priceIds` (with the same order).
function parsePriceFeedUpdates(
bytes[] calldata updateData,
bytes32[] calldata priceIds,
uint64 minPublishTime,
uint64 maxPublishTime
) external payable returns (PythStructs.PriceFeed[] memory priceFeeds);
}
contract C {
IPyth pyth;
constructor(IPyth _pyth) {
pyth = _pyth;
}
function bad(bytes32 id, uint256 age) public {
PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age);
require(price.publishTime > block.timestamp - 120);
// Use price
}
function good(bytes32 id, uint256 age) public {
PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age);
require(price.conf < 10000);
require(price.publishTime > block.timestamp - 120);
// Use price
}
function good2(bytes32 id, uint256 age) public {
PythStructs.Price memory price = pyth.getEmaPriceNoOlderThan(id, age);
require(price.publishTime > block.timestamp - 120);
if (price.conf >= 10000) {
revert();
}
// Use price
}
}

@ -1714,6 +1714,16 @@ ALL_TESTS = [
"out_of_order_retryable.sol", "out_of_order_retryable.sol",
"0.8.20", "0.8.20",
), ),
Test(
all_detectors.PythUncheckedConfidence,
"pyth_unchecked_confidence.sol",
"0.8.20",
),
Test(
all_detectors.PythUncheckedPublishTime,
"pyth_unchecked_publishtime.sol",
"0.8.20",
),
# Test( # Test(
# all_detectors.UnusedImport, # all_detectors.UnusedImport,
# "ConstantContractLevelUsedInContractTest.sol", # "ConstantContractLevelUsedInContractTest.sol",

Loading…
Cancel
Save