Fix Storage and calldata

pull/1076/head
Nikhil Parasaram 6 years ago
parent c755a58dc4
commit b3bf6fbcde
  1. 56
      mythril/laser/ethereum/instructions.py
  2. 16
      mythril/laser/ethereum/state/account.py
  3. 4
      tests/laser/evm_testsuite/evm_test.py
  4. 12
      tests/laser/state/storage_test.py
  5. 4
      tests/testdata/outputs_expected/metacoin.sol.o.graph.html
  6. 16
      tests/testdata/outputs_expected/metacoin.sol.o.json
  7. 20
      tests/testdata/outputs_expected/metacoin.sol.o.jsonv2
  8. 15
      tests/testdata/outputs_expected/metacoin.sol.o.markdown
  9. 12
      tests/testdata/outputs_expected/metacoin.sol.o.text

@ -768,18 +768,10 @@ class Instruction:
size_sym = True size_sym = True
if size_sym: if size_sym:
state.mem_extend(mstart, 1) size = (
state.memory[mstart] = global_state.new_bitvec( 320
"calldata_" ) # This excess stuff will get overwritten as memory is dynamically sized
+ str(environment.active_account.contract_name)
+ "["
+ str(dstart)
+ ": + "
+ str(size)
+ "]",
8,
)
return [global_state]
size = cast(int, size) size = cast(int, size)
if size > 0: if size > 0:
try: try:
@ -1389,7 +1381,15 @@ class Instruction:
return self._sload_helper(global_state, str(index)) return self._sload_helper(global_state, str(index))
storage_keys = global_state.environment.active_account.storage.keys() storage_keys = global_state.environment.active_account.storage.keys()
keccak_keys = list(filter(keccak_function_manager.is_keccak, storage_keys)) keys = filter(keccak_function_manager.is_keccak, storage_keys)
addr = global_state.get_current_instruction()["address"]
keccak_keys = [
key
for key in keys
if global_state.environment.active_account.storage.potential_func(
key, addr
)
]
results = [] # type: List[GlobalState] results = [] # type: List[GlobalState]
constraints = [] constraints = []
@ -1427,11 +1427,16 @@ class Instruction:
:param constraints: :param constraints:
:return: :return:
""" """
address = global_state.get_current_instruction()["address"]
try: try:
data = global_state.environment.active_account.storage[index] data = global_state.environment.active_account.storage.get(
index, addr=address
)
except KeyError: except KeyError:
data = global_state.new_bitvec("storage_" + str(index), 256) data = global_state.new_bitvec("storage_" + str(index), 256)
global_state.environment.active_account.storage[index] = data global_state.environment.active_account.storage.put(
key=index, value=data, addr=address
)
if constraints is not None: if constraints is not None:
global_state.mstate.constraints += constraints global_state.mstate.constraints += constraints
@ -1475,7 +1480,15 @@ class Instruction:
return self._sstore_helper(global_state, str(index), value) return self._sstore_helper(global_state, str(index), value)
storage_keys = global_state.environment.active_account.storage.keys() storage_keys = global_state.environment.active_account.storage.keys()
keccak_keys = filter(keccak_function_manager.is_keccak, storage_keys) keccak_keys = list(filter(keccak_function_manager.is_keccak, storage_keys))
addr = global_state.get_current_instruction()["address"]
keccak_keys = [
key
for key in keccak_keys
if global_state.environment.active_account.storage.potential_func(
key, addr
)
]
results = [] # type: List[GlobalState] results = [] # type: List[GlobalState]
new = symbol_factory.Bool(False) new = symbol_factory.Bool(False)
@ -1528,19 +1541,18 @@ class Instruction:
:param constraint: :param constraint:
:return: :return:
""" """
try:
global_state.environment.active_account = deepcopy( global_state.environment.active_account = deepcopy(
global_state.environment.active_account global_state.environment.active_account
) )
global_state.accounts[ global_state.accounts[
global_state.environment.active_account.address.value global_state.environment.active_account.address.value
] = global_state.environment.active_account ] = global_state.environment.active_account
address = global_state.get_current_instruction()["address"]
global_state.environment.active_account.storage[index] = ( global_state.environment.active_account.storage.put(
value if not isinstance(value, Expression) else simplify(value) key=index,
value=value if not isinstance(value, Expression) else simplify(value),
addr=address,
) )
except KeyError:
log.debug("Error writing to storage: Invalid index")
if constraint is not None: if constraint is not None:
global_state.mstate.constraints.append(constraint) global_state.mstate.constraints.append(constraint)

@ -24,8 +24,12 @@ class Storage:
self.concrete = concrete self.concrete = concrete
self.dynld = dynamic_loader self.dynld = dynamic_loader
self.address = address self.address = address
self._storage_opcodes = {} # type: Dict
def __getitem__(self, item: Union[str, int]) -> Any: def get(self, item: Union[str, int], addr: int) -> Any:
if item not in self._storage_opcodes:
self._storage_opcodes[item] = set()
self._storage_opcodes[item].add(addr)
try: try:
return self._storage[item] return self._storage[item]
except KeyError: except KeyError:
@ -56,9 +60,17 @@ class Storage:
) )
return self._storage[item] return self._storage[item]
def __setitem__(self, key: Union[int, str], value: Any) -> None: def put(self, key: Union[int, str], value: Any, addr) -> None:
if key not in self._storage_opcodes:
self._storage_opcodes[key] = set()
self._storage_opcodes[key].add(addr)
self._storage[key] = value self._storage[key] = value
def potential_func(self, key, opcode) -> bool:
if key not in self._storage_opcodes:
return False
return opcode in self._storage_opcodes[key]
def keys(self) -> KeysView: def keys(self) -> KeysView:
""" """

@ -125,7 +125,7 @@ def test_vmtest(
account.code = Disassembly(details["code"][2:]) account.code = Disassembly(details["code"][2:])
account.nonce = int(details["nonce"], 16) account.nonce = int(details["nonce"], 16)
for key, value in details["storage"].items(): for key, value in details["storage"].items():
account.storage[int(key, 16)] = int(value, 16) account.storage.put(int(key, 16), int(value, 16), 10)
world_state.put_account(account) world_state.put_account(account)
account.set_balance(int(details["balance"], 16)) account.set_balance(int(details["balance"], 16))
@ -175,7 +175,7 @@ def test_vmtest(
for index, value in details["storage"].items(): for index, value in details["storage"].items():
expected = int(value, 16) expected = int(value, 16)
actual = account.storage[int(index, 16)] actual = account.storage.get(int(index, 16), 0)
if isinstance(actual, Expression): if isinstance(actual, Expression):
actual = actual.value actual = actual.value

@ -12,7 +12,7 @@ def test_concrete_storage_uninitialized_index(initial_storage, key):
storage._storage = initial_storage storage._storage = initial_storage
# Act # Act
value = storage[key] value = storage.get(key, 0)
# Assert # Assert
assert value == 0 assert value == 0
@ -25,7 +25,7 @@ def test_symbolic_storage_uninitialized_index(initial_storage, key):
storage._storage = initial_storage storage._storage = initial_storage
# Act # Act
value = storage[key] value = storage.get(key, 0)
# Assert # Assert
assert isinstance(value, Expression) assert isinstance(value, Expression)
@ -36,10 +36,10 @@ def test_storage_set_item():
storage = Storage() storage = Storage()
# Act # Act
storage[1] = 13 storage.put(key=1, value=13, addr=10)
# Assert # Assert
assert storage[1] == 13 assert storage.put(key=1, value=13, addr=10)
def test_storage_change_item(): def test_storage_change_item():
@ -47,7 +47,7 @@ def test_storage_change_item():
storage = Storage() storage = Storage()
storage._storage = {1: 12} storage._storage = {1: 12}
# Act # Act
storage[1] = 14 storage.put(key=1, value=14, addr=10)
# Assert # Assert
assert storage[1] == 14 assert storage.get(item=1, addr=10) == 14

File diff suppressed because one or more lines are too long

@ -1,5 +1,19 @@
{ {
"error": null, "error": null,
"issues": [], "issues": [
{
"address": 498,
"contract": "Unknown",
"debug": "<DEBUG-DATA>",
"description": "The binary addition can overflow.\nThe operands of the addition operation are not sufficiently constrained. The addition could therefore result in an integer overflow. Prevent the overflow by checking inputs or ensure sure that the overflow is caught by an assertion.",
"function": "sendToken(address,uint256)",
"max_gas_used": 52806,
"min_gas_used": 11860,
"severity": "High",
"sourceMap": null,
"swc-id": "101",
"title": "Integer Overflow"
}
],
"success": true "success": true
} }

@ -1,6 +1,24 @@
[ [
{ {
"issues": [], "issues": [
{
"description": {
"head": "The binary addition can overflow.",
"tail": "The operands of the addition operation are not sufficiently constrained. The addition could therefore result in an integer overflow. Prevent the overflow by checking inputs or ensure sure that the overflow is caught by an assertion."
},
"extra": {
"discoveryTime": "<DISCOVERY-TIME-DATA>"
},
"locations": [
{
"sourceMap": "498:1:0"
}
],
"severity": "High",
"swcID": "SWC-101",
"swcTitle": "Integer Overflow and Underflow"
}
],
"meta": {}, "meta": {},
"sourceFormat": "evm-byzantium-bytecode", "sourceFormat": "evm-byzantium-bytecode",
"sourceList": [ "sourceList": [

@ -1,3 +1,14 @@
# Analysis results for None # Analysis results for test-filename.sol
The analysis was completed successfully. No issues were detected. ## Integer Overflow
- SWC ID: 101
- Severity: High
- Contract: Unknown
- Function name: `sendToken(address,uint256)`
- PC address: 498
- Estimated Gas Usage: 11860 - 52806
### Description
The binary addition can overflow.
The operands of the addition operation are not sufficiently constrained. The addition could therefore result in an integer overflow. Prevent the overflow by checking inputs or ensure sure that the overflow is caught by an assertion.

@ -1 +1,11 @@
The analysis was completed successfully. No issues were detected. ==== Integer Overflow ====
SWC ID: 101
Severity: High
Contract: Unknown
Function name: sendToken(address,uint256)
PC address: 498
Estimated Gas Usage: 11860 - 52806
The binary addition can overflow.
The operands of the addition operation are not sufficiently constrained. The addition could therefore result in an integer overflow. Prevent the overflow by checking inputs or ensure sure that the overflow is caught by an assertion.
--------------------

Loading…
Cancel
Save