Skip to content

Commit

Permalink
Detecting the use of BALANCE in a strict comparison (#1481)
Browse files Browse the repository at this point in the history
* Detecting the use of BALANCE in a strict comparison

* Black

* mock tst

* blackeningin

* Fix typo
  • Loading branch information
feliam authored and Eric Hennenfent committed Aug 5, 2019
1 parent 0f1a4f9 commit d06d5aa
Show file tree
Hide file tree
Showing 13 changed files with 150 additions and 21 deletions.
12 changes: 7 additions & 5 deletions manticore/core/smtlib/expression.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,14 @@ def get_taints(arg, taint=None):
return


def taint_with(arg, taint, value_bits=256, index_bits=256):
def taint_with(arg, *taints, value_bits=256, index_bits=256):
"""
Helper to taint a value.
:param arg: a value or Expression
:param taint: a regular expression matching a taint value (eg. 'IMPORTANT.*'). If None, this function checks for any taint value.
"""

tainted_fset = frozenset((taint,))

tainted_fset = frozenset(tuple(taints))
if not issymbolic(arg):
if isinstance(arg, int):
arg = BitVecConstant(value_bits, arg)
Expand All @@ -93,8 +92,11 @@ def taint_with(arg, taint, value_bits=256, index_bits=256):
raise ValueError("type not supported")

else:
arg = copy.copy(arg)
arg._taint |= tainted_fset
if isinstance(arg, BitVecVariable):
arg = arg + BitVecConstant(value_bits, 0, taint=tainted_fset)
else:
arg = copy.copy(arg)
arg._taint |= tainted_fset

return arg

Expand Down
11 changes: 10 additions & 1 deletion manticore/core/smtlib/solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from ...exceptions import Z3NotFoundError, SolverError, SolverUnknown, TooManySolutions
from ...utils import config
from . import issymbolic
import time

logger = logging.getLogger(__name__)
consts = config.get_group("smt")
Expand Down Expand Up @@ -433,6 +434,7 @@ def get_all_values(self, constraints, expression, maxcnt=None, silent=False):

result = []

start = time.time()
while self._is_sat():
value = self._getvalue(var)
result.append(value)
Expand All @@ -447,7 +449,8 @@ def get_all_values(self, constraints, expression, maxcnt=None, silent=False):
break
else:
raise TooManySolutions(result)

if time.time() - start > consts.timeout:
raise SolverError("Timeout")
return result

def optimize(self, constraints: ConstraintSet, x: BitVec, goal: str, M=10000):
Expand All @@ -471,6 +474,7 @@ def optimize(self, constraints: ConstraintSet, x: BitVec, goal: str, M=10000):
self._reset(temp_cs.to_string(related_to=X))
self._send(aux.declaration)

start = time.time()
if getattr(self, f"support_{goal}"):
self._push()
try:
Expand Down Expand Up @@ -511,6 +515,8 @@ def optimize(self, constraints: ConstraintSet, x: BitVec, goal: str, M=10000):
i = i + 1
if i > M:
raise SolverError("Optimizing error, maximum number of iterations was reached")
if time.time() - start > consts.timeout:
raise SolverError("Timeout")
if last_value is not None:
return last_value
raise SolverError("Optimizing error, unsat or unknown core")
Expand All @@ -522,6 +528,7 @@ def get_value(self, constraints, expression):
if not issymbolic(expression):
return expression
assert isinstance(expression, (Bool, BitVec, Array))
start = time.time()
with constraints as temp_cs:
if isinstance(expression, Bool):
var = temp_cs.new_bool()
Expand All @@ -547,6 +554,8 @@ def get_value(self, constraints, expression):
m = pattern.match(ret)
expr, value = m.group("expr"), m.group("value")
result.append(int(value, base))
if time.time() - start > consts.timeout:
raise SolverError("Timeout")
return bytes(result)

temp_cs.add(var == expression)
Expand Down
6 changes: 3 additions & 3 deletions manticore/core/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Concretize(StateException):
"""

_ValidPolicies = ["MINMAX", "ALL", "SAMPLED", "ONE"]
_ValidPolicies = ["MIN", "MAX", "MINMAX", "ALL", "SAMPLED", "ONE"]

def __init__(self, message, expression, setstate=None, policy=None, **kwargs):
if policy is None:
Expand Down Expand Up @@ -247,9 +247,9 @@ def concretize(self, symbolic, policy, maxcount=7):
if policy == "MINMAX":
vals = self._solver.minmax(self._constraints, symbolic)
elif policy == "MAX":
vals = self._solver.max(self._constraints, symbolic)
vals = (self._solver.max(self._constraints, symbolic),)
elif policy == "MIN":
vals = self._solver.min(self._constraints, symbolic)
vals = (self._solver.min(self._constraints, symbolic),)
elif policy == "SAMPLED":
m, M = self._solver.minmax(self._constraints, symbolic)
vals += [m, M]
Expand Down
2 changes: 1 addition & 1 deletion manticore/core/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def run(self, *args):
assert current_state is None
# Handling Forking and terminating exceptions
except Concretize as exc:
logger.debug("[%r] Debug %r", self.id, exc)
logger.info("[%r] Debug %r", self.id, exc)
# The fork() method can decides which state to keep
# exploring. For example when the fork results in a
# single state it is better to just keep going.
Expand Down
1 change: 1 addition & 0 deletions manticore/ethereum/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
DetectUninitializedMemory,
DetectUninitializedStorage,
DetectRaceCondition,
DetectManipulableBalance,
)
from .account import EVMAccount, EVMContract
from .abi import ABI
Expand Down
2 changes: 2 additions & 0 deletions manticore/ethereum/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
DetectEnvInstruction,
DetectRaceCondition,
DetectorClassification,
DetectManipulableBalance,
)
from ..core.plugin import Profiler
from .manticore import ManticoreEVM
Expand All @@ -36,6 +37,7 @@ def get_detectors_classes():
DetectDelegatecall,
DetectExternalCallAndLeak,
DetectEnvInstruction,
DetectManipulableBalance,
# The RaceCondition detector has been disabled for now as it seems to collide with IntegerOverflow detector
# DetectRaceCondition
]
Expand Down
26 changes: 26 additions & 0 deletions manticore/ethereum/detectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ def add_finding(self, state, address, pc, finding, at_init, constraint=True):
self.get_findings(state).append((address, pc, finding, at_init, constraint))
with self.locked_global_findings() as gf:
gf.append((address, pc, finding, at_init))

# Fixme for ever broken logger
logger.warning(finding)

Expand Down Expand Up @@ -838,3 +839,28 @@ def did_evm_execute_instruction_callback(self, state, instruction, arguments, re

self.__findings.add(unique_key)
self.add_finding_here(state, msg)


class DetectManipulableBalance(Detector):
"""
Detects the use of manipulable balance in strict compare.
"""

ARGUMENT = "lockdrop"
HELP = "Use balance in EQ"
IMPACT = DetectorClassification.HIGH
CONFIDENCE = DetectorClassification.HIGH

def did_evm_execute_instruction_callback(self, state, instruction, arguments, result):
vm = state.platform.current_vm
mnemonic = instruction.semantics

if mnemonic == "BALANCE":
# replace result with tainted value
result = taint_with(result, "BALANCE")
vm.change_last_result(result)
elif mnemonic == "EQ":
# check if balance tainted
for op in arguments:
if istainted(op, "BALANCE"):
self.add_finding_here(state, "Manipulable balance used in a strict comparison")
7 changes: 6 additions & 1 deletion manticore/ethereum/manticore.py
Original file line number Diff line number Diff line change
Expand Up @@ -789,13 +789,15 @@ def solidity_create_contract(
self.kill()
raise
except Exception as e:
logger.info("Failed to compile contract", str(e))
self.kill()
raise

# If the contract was created successfully in at least 1 state return account
for state in self.ready_states:
if state.platform.get_code(int(contract_account)):
return contract_account
logger.info("Failed to compile contract", contract_names)
return None

def get_nonce(self, address):
Expand Down Expand Up @@ -1648,7 +1650,10 @@ def finalizer(state_id):
logger.debug("Generating testcase for state_id %d", state_id)
last_tx = st.platform.last_transaction
message = last_tx.result if last_tx else "NO STATE RESULT (?)"
self.generate_testcase(st, message=message)
try:
self.generate_testcase(st, message=message)
except Exception as e:
print(e)

def worker_finalize(q):
try:
Expand Down
43 changes: 35 additions & 8 deletions manticore/platforms/evm.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@
),
)

consts.add(
"calldata_max_offset",
default=1024 * 1024,
description="Max calldata offset to explore with. Iff offset or size in a calldata related instruction are symbolic it will be constrained to this constant",
)

# Auxiliary constants and functions
TT256 = 2 ** 256
TT256M1 = 2 ** 256 - 1
Expand Down Expand Up @@ -543,7 +549,9 @@ def wrapper(*args, **kwargs):
f"Concretizing {func.__name__}'s {index} argument and dropping its taints: "
"the value might not be tracked properly (in case of using detectors)"
)
logger.info(f"Concretizing instruction argument {arg} by {policy}")
logger.info(
f"Concretizing instruction {args[0].world.current_vm.instruction} argument {arg} by {policy}"
)
raise ConcretizeArgument(index, policy=policy)
return func(*args, **kwargs)

Expand Down Expand Up @@ -1200,7 +1208,7 @@ def setstate(state, value):
raise Concretize(
"Concretize PC", expression=expression, setstate=setstate, policy="ALL"
)
# print (self)
# print(self.instruction)
try:
# import time
# limbo = 0.0
Expand Down Expand Up @@ -1603,7 +1611,9 @@ def CALLDATALOAD(self, offset):
if issymbolic(offset):
if Z3Solver().can_be_true(self._constraints, offset == self._used_calldata_size):
self.constraints.add(offset == self._used_calldata_size)
raise ConcretizeArgument(1, policy="SAMPLED")
offset = self._used_calldata_size
else:
raise ConcretizeArgument(1, policy="SAMPLED")

self._use_calldata(offset, 32)

Expand Down Expand Up @@ -1644,18 +1654,36 @@ def CALLDATACOPY_gas(self, mem_offset, data_offset, size):
def CALLDATACOPY(self, mem_offset, data_offset, size):
"""Copy input data in current environment to memory"""
if issymbolic(size):
if Z3Solver().can_be_true(self._constraints, size <= len(self.data) + 32):
self.constraints.add(size <= len(self.data) + 32)
if not Z3Solver().can_be_true(
self._constraints, Operators.ULE(size, len(self.data) + data_offset + 32)
):
print("omg it can not be small")
import pdb

pdb.set_trace()
raise ConcretizeArgument(3, policy="MIN")
self.constraints.add(Operators.ULE(size, len(self.data) + data_offset + 32))
raise ConcretizeArgument(3, policy="SAMPLED")

if issymbolic(data_offset):
if Z3Solver().can_be_true(self._constraints, data_offset == self._used_calldata_size):
self.constraints.add(data_offset == self._used_calldata_size)
raise ConcretizeArgument(2, policy="SAMPLED")
data_offset = self._used_calldata_size
print("symbolic data_offset", data_offset, "choosing", self._used_calldata_size)
else:
logger.debug("symbolic data_offset MIN")
raise ConcretizeArgument(2, policy="MIN")

# account for calldata usage
self._use_calldata(data_offset, size)
self._allocate(mem_offset, size)
if size > consts.calldata_max_offset:
logger.info("CALLDATACOPY absurd size %d. OOG policy used: %r", size, consts.oog)
raise TerminateState(
"CALLDATACOPY absurd size %d. OOG policy used: %r" % (size, consts.oog),
testcase=True,
)

for i in range(size):
try:
c = Operators.ITEBV(
Expand Down Expand Up @@ -1808,8 +1836,7 @@ def MSTORE_gas(self, address, value):
def MSTORE(self, address, value):
"""Save word to memory"""
if istainted(self.pc):
for taint in get_taints(self.pc):
value = taint_with(value, taint)
value = taint_with(value, *get_taints(self.pc))
self._allocate(address, 32)
self._store(address, value, 32)

Expand Down
12 changes: 12 additions & 0 deletions tests/ethereum/contracts/detectors/balance_not_ok.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
contract DetectThis {
event Log(string);
function foo() payable public{
uint256 eth2;
uint256 eth = msg.value;
eth2 = address(msg.sender).balance;
if(eth2 == eth){
emit Log("Found a bug");
}
}
}

12 changes: 12 additions & 0 deletions tests/ethereum/contracts/detectors/balance_ok.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
contract DetectThis {
event Log(string);
function foo() payable public{
uint256 eth2;
uint256 eth = msg.value;
eth2 = address(msg.sender).balance;
if(eth2 >= eth){
emit Log("Found a bug");
}
}
}

15 changes: 15 additions & 0 deletions tests/ethereum/test_detectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
DetectExternalCallAndLeak,
DetectEnvInstruction,
DetectRaceCondition,
DetectManipulableBalance,
State,
)
from manticore.ethereum.plugins import LoopDepthLimiter
Expand Down Expand Up @@ -394,5 +395,19 @@ def test_race_condition2(self):
)


class EthBalance(EthDetectorTest):
""" Test the detection of funny delegatecalls """

DETECTOR_CLASS = DetectManipulableBalance

def test_balance_ok(self):
name = inspect.currentframe().f_code.co_name[5:]
self._test(name, set())

def test_balance_not_ok(self):
name = inspect.currentframe().f_code.co_name[5:]
self._test(name, {(("Manipulable balance used in a strict comparison", False))})


if __name__ == "__main__":
unittest.main()
22 changes: 20 additions & 2 deletions tests/ethereum/test_general.py
Original file line number Diff line number Diff line change
Expand Up @@ -1313,8 +1313,17 @@ def test_concretizer(self):
def inner_func(self, a, b):
return a, b

class X:
class Y:
class Z:
instruction = "instruction"

current_vm = Z()

world = Y()

with self.assertRaises(ConcretizeArgument) as cm:
inner_func(None, self.bv, 34)
inner_func(X(), self.bv, 34)

self.assertEqual(cm.exception.pos, 1)
self.assertEqual(cm.exception.policy, policy)
Expand All @@ -1324,8 +1333,17 @@ def test_concretizer_default(self):
def inner_func(self, a, b):
return a, b

class X:
class Y:
class Z:
instruction = "instruction"

current_vm = Z()

world = Y()

with self.assertRaises(ConcretizeArgument) as cm:
inner_func(None, 34, self.bv)
inner_func(X(), 34, self.bv)

self.assertEqual(cm.exception.pos, 2)
# Make sure the policy isn't blank, i.e. we didn't pass through
Expand Down

0 comments on commit d06d5aa

Please sign in to comment.