add detector

docs/price_manipulation.md

@@ -0,0 +1,15 @@
+# Price Manipulation through token transfers
+
+## Configuration
+* Check: `pess-price-manipulation`
+* Severity: `High`
+* Confidence: `Low`
+
+## Description
+The detector finds calculations that depend on the balance and supply of some token. Such calculations could be manipulated through direct transfers to the contract, increasing its balance.
+
+## Vulnerable Scenario
+[test scenario](../tests/price_manipulation_test.sol)
+
+## Recommendation
+Avoid possible manipulations of calculations because of external transfers.

slitherin/__init__.py

@@ -32,6 +32,7 @@
 from slitherin.detectors.arbitrum.arbitrum_chainlink_price_feed import ArbitrumChainlinkPriceFeed
 from slitherin.detectors.potential_arith_overflow import PotentialArithmOverflow
 from slitherin.detectors.curve.curve_readonly_reentrancy import CurveReadonlyReentrancy
+from slitherin.detectors.price_manipulation import PriceManipulationDetector
 
 artbitrum_detectors = [
  ArbitrumPrevrandaoDifficulty,
@@ -65,6 +66,7 @@
  AAVEFlashloanCallbackDetector,
  PotentialArithmOverflow,
  CurveReadonlyReentrancy,
+ PriceManipulationDetector
 ]
 plugin_printers = []
 

slitherin/detectors/price_manipulation.py

@@ -3,35 +3,67 @@
 from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
 from slither.core.declarations import Function
 from slither.slithir.operations.event_call import EventCall
+from slither.slithir.operations.high_level_call import HighLevelCall
+from slither.slithir.operations.internal_call import InternalCall
+from slither.slithir.operations.binary import Binary, BinaryType
+from slither.analyses.data_dependency.data_dependency import is_dependent
 
 
 class PriceManipulationDetector(AbstractDetector):
- """
- Sees if contract setters do not emit events
- """
-
  ARGUMENT = 'pess-price-manipulation' # slither will launch the detector with slither.py --detect mydetector
- HELP = 'Contract function does not emit event after the value is set'
- IMPACT = DetectorClassification.LOW
- CONFIDENCE = DetectorClassification.MEDIUM
+ HELP = 'Contract math can be manipulated through external token transfers.'
+ IMPACT = DetectorClassification.HIGH
+ CONFIDENCE = DetectorClassification.LOW
 
- WIKI = 'https://github.com/pessimistic-io/slitherin/blob/master/docs/event_setter.md'
- WIKI_TITLE = 'Missing Event Setter'
- WIKI_DESCRIPTION = "Setter-functions must emit events"
+ WIKI = 'https://github.com/pessimistic-io/slitherin/blob/master/docs/price_manipulation.md'
+ WIKI_TITLE = '# Price Manipulation through token transfers'
+ WIKI_DESCRIPTION = "Calculations could be manipulated through direct transfers to the contract, increasing its balance as they depends on these balances."
  WIKI_EXPLOIT_SCENARIO = 'N/A'
- WIKI_RECOMMENDATION = 'Emit events in setter functions'
+ WIKI_RECOMMENDATION = 'Avoid possible manipulations of calculations because of external transfers.'
 
  def _detect(self) -> List[Output]:
- """Main function"""
- res = []
- for contract in self.compilation_unit.contracts_derived:
+ all_balance_vars = []
+ all_supply_vars = []
+ all_binary_ops = []
+ for contract in self.contracts:
  if not contract.is_interface:
- for f in contract.functions_and_modifiers_declared:
- if f.name.startswith("set"):
- x = self._emits_event(f)
- if not x:
- res.append(self.generate_result([
- "Setter function ",
- f, ' does not emit an event'
- '\n']))
- return res
+ for func in contract.functions:
+ for n in func.nodes:
+ for x in n.irs:
+ if isinstance(x, HighLevelCall):
+ if str(x.function_name).lower() == "balanceof":
+ all_balance_vars.append((n, x._lvalue))
+ if "supply" in str(x.function_name).lower():
+ all_supply_vars.append((n, x._lvalue))
+ if isinstance(x, InternalCall):
+ if "supply" in str(x.function_name).lower():
+ all_supply_vars.append((n, x._lvalue))
+ if isinstance(x, Binary):
+ all_binary_ops.append((n, x))
+ results = []
+ for (balance_node, bal) in all_balance_vars:
+ for (supply_node, supply) in all_supply_vars:
+ for (node, bin_op) in all_binary_ops:
+ l, r = bin_op.get_variable
+ is_bal_dependent = is_dependent(l, bal, contract)
+ is_supply_dependent = is_dependent(r, supply, contract)
+ if is_bal_dependent and is_supply_dependent:
+ results.append((node, balance_node, supply_node))
+ if not results:
+ return []
+
+
+ response = []
+ for issue_node, balance_node, supply_node in results:
+ res = []
+ res.append("Calculation ")
+ res.append(issue_node)
+ res.append(" depends on balance and token supply - these values could be manipulated through external calls.\n")
+ res.append("\tBalance dependency: ")
+ res.append(balance_node)
+ res.append("\n")
+ res.append("\tSupply dependency: ")
+ res.append(supply_node)
+ res.append("\n")
+ response.append(self.generate_result(res))
+ return response

tests/price_manipulation_test.sol

@@ -0,0 +1,43 @@
+interface IERC20 {
+ function totalSupply() external view returns (uint256);
+ function balanceOf(address account) external view returns (uint256);
+}
+contract Test1 {
+ IERC20 token;
+
+ function test_vuln_1() external returns(uint256 price) {
+ price = token.balanceOf(address(this)) / token.totalSupply();
+ }
+
+ function test_vuln_2() external returns(uint256 price) {
+ uint256 bal = token.balanceOf(address(this));
+ uint256 supply = token.totalSupply();
+ price = bal / supply; 
+ }
+
+ function test_vuln_3() external returns(uint256 price) {
+ uint256 bal = getBalance();
+ price = bal / token.totalSupply();
+ }
+
+ function test_vuln_4() external returns(uint256 price) {
+ uint256 bal = getBalance();
+ price = 10 + (bal / (token.totalSupply() * 5));
+ }
+
+ function test_vuln_5() external returns(uint256 price) {
+ price = getBalance() / mySupply();
+ }
+
+ function test_vuln_6() external returns(uint256 price) {
+ price = getBalance() + mySupply() + 1;
+ }
+
+ function getBalance() public returns(uint256 bal) {
+ bal = token.balanceOf(msg.sender);
+ }
+
+ function mySupply() public returns (uint256) {
+ return 100;
+ }
+}