Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix eth private func calls #1306

Merged
merged 6 commits into from
Dec 18, 2018
Merged

Fix eth private func calls #1306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
485cb82
Add test for calling contract._func
disconnect3d Dec 17, 2018
03c3e38
Fix contract._func calls
disconnect3d Dec 17, 2018
555ae60
EVMContract: make attributes protected
disconnect3d Dec 17, 2018
502fca9
Move EVMContract __hashes={} to __init__
disconnect3d Dec 17, 2018
095ebae
Fix attribute error
disconnect3d Dec 17, 2018
9cf1de4
Review fix
disconnect3d Dec 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 79 additions & 80 deletions manticore/ethereum/account.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
HashesEntry = namedtuple('HashesEntry', 'signature func_id')


class EVMAccount(object):
class EVMAccount:
def __init__(self, address=None, manticore=None, name=None):
""" Encapsulates an account.

:param address: the address of this account
:type address: 160 bit long integer
:param manticore: the controlling Manticore
"""
Encapsulates an account.

:param address: the address of this account
:type address: 160 bit long integer
:param manticore: the controlling Manticore
"""
self._manticore = manticore
self._address = address
Expand All @@ -31,8 +31,8 @@ def __eq__(self, other):
@property
def name_(self):
"""
This is named this way to avoid naming collisions with Solidity functions/data, since EVMContract inherits
this.
This is named this way to avoid naming collisions with Solidity functions/data,
since EVMContract inherits this.
"""
return self._name

Expand All @@ -48,93 +48,92 @@ def __str__(self):


class EVMContract(EVMAccount):
""" An EVM account

Note: The private methods of this class begin with a double underscore to
avoid function name collisions with Solidity functions that begin with
a single underscore
"""
An EVM account

Note: The private methods of this class begin with a double underscore to avoid function
name collisions with Solidity functions that begin with a single underscore.
"""
def __init__(self, default_caller=None, **kwargs):
""" Encapsulates a contract account.
:param default_caller: the default caller address for any transaction
"""
Encapsulates a contract account.

:param default_caller: the default caller address for any transaction
"""
super().__init__(**kwargs)
self._default_caller = default_caller
self._hashes = None
self.__default_caller = default_caller
self.__hashes = {}
self.__initialized = False

def add_function(self, signature):
func_id = ABI.function_selector(signature)
func_name = str(signature.split('(')[0])
if func_name.startswith('__') or func_name in {'add_function', 'address', 'name_'}:
# TODO(mark): is this actually true? is there anything actually wrong with a solidity name beginning w/ an underscore?
raise EthereumError("Function name ({}) is internally reserved".format(func_name))
entry = HashesEntry(signature, func_id)
if func_name in self._hashes:
self._hashes[func_name].append(entry)
return
if func_id in {entry.func_id for entries in self._hashes.values() for entry in entries}:
raise EthereumError("A function with the same hash as {} is already defined".format(func_name))
self._hashes[func_name] = [entry]

def __null_func(self):
pass
if func_name in self.__dict__ or func_name in {'add_function', 'address', 'name_'}:
raise EthereumError(f"Function name ({func_name}) is internally reserved")

def __init_hashes(self):
#initializes self._hashes lazy
if self._hashes is None and self._manticore is not None:
self._hashes = {}
md = self._manticore.get_metadata(self._address)
if md is not None:
for signature in md.function_signatures:
self.add_function(signature)
# It was successful, no need to re-run. _init_hashes disabled
self.__init_hashes = self.__null_func
entry = HashesEntry(signature, func_id)

def __getattribute__(self, name):
""" If this is a contract account of which we know the functions hashes,
this will build the transaction for the function call.
if func_name in self.__hashes:
self.__hashes[func_name].append(entry)
return

Example use::
if func_id in {entry.func_id for entries in self.__hashes.values() for entry in entries}:
raise EthereumError(f"A function with the same hash as {func_name} is already defined")

#call function `add` on contract_account with argument `1000`
contract_account.add(1000)
self.__hashes[func_name] = [entry]

def __init_hashes(self):
md = self._manticore.get_metadata(self._address)
if md is not None:
for signature in md.function_signatures:
self.add_function(signature)
# It was successful, no need to re-run. _init_hashes disabled
self.__initialized = True

def __getattr__(self, name):
disconnect3d marked this conversation as resolved.
Show resolved Hide resolved
"""
if not name.startswith('_'):
If this is a contract account of which we know the functions hashes,
this will build the transaction for the function call.

Example use:
# call function `add` on contract_account with argument `1000`
contract_account.add(1000)
"""
if not self.__initialized:
self.__init_hashes()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor concern, calling __init_hashes does not necessarily guarantee that self._hashes is initialized to a dict, so the below name in self._hashes check could crash. this would only happen is self._manticore was None which probably won't happen, but still wanted to mention it. may be worth keeping the self._hashes is not None check.

alternate idea. simply initialize _hashes to an empty dict? not exactly clear on why we need to separately init it to None, and then to a dict.

third idea. we modify __init_hashes to ensure that _hashes is ALWAYS set to a dict for all paths through it, then we don't need the is not None check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I think we can assume it is initialized. I will add an assertion to EVMAccount whether self._manticore is a ManticoreEVM instance.

Copy link
Member Author

@disconnect3d disconnect3d Dec 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think that self.__hashes = {} should be in __init__.

if self._hashes is not None and name in self._hashes.keys():
def f(*args, signature: Optional[str]=None, caller=None, value=0, gas=0xffffffffffff, **kwargs):
try:
if signature:
if f'{name}{signature}' not in {entry.signature for entries in self._hashes.values() for entry in entries}:
raise EthereumError(
f'Function: `{name}` has no such signature`\n'
f'Known signatures: {[entry.signature[len(name):] for entry in self._hashes[name]]}')

tx_data = ABI.function_call(f'{name}{signature}', *args)
else:
entries = self._hashes[name]
if len(entries) > 1:
sig = entries[0].signature[len(name):]
raise EthereumError(
f'Function: `{name}` has multiple signatures but `signature` is not '
f'defined! Example: `account.{name}(..., signature="{sig}")`\n'
f'Known signatures: {[entry.signature[len(name):] for entry in self._hashes[name]]}')

tx_data = ABI.function_call(str(entries[0].signature), *args)
except KeyError as e:
raise e

if caller is None:
caller = self._default_caller

self._manticore.transaction(caller=caller,
address=self._address,
value=value,
data=tx_data,
gas=gas)
return f

return object.__getattribute__(self, name)

if name in self.__hashes:
def f(*args, signature: Optional[str]=None, caller=None, value=0, gas=0xffffffffffff, **kwargs):
try:
if signature:
if f'{name}{signature}' not in {entry.signature for entries in self.__hashes.values() for entry in entries}:
raise EthereumError(
f'Function: `{name}` has no such signature\n'
f'Known signatures: {[entry.signature[len(name):] for entry in self.__hashes[name]]}')

tx_data = ABI.function_call(f'{name}{signature}', *args)
else:
entries = self.__hashes[name]
if len(entries) > 1:
sig = entries[0].signature[len(name):]
raise EthereumError(
f'Function: `{name}` has multiple signatures but `signature` is not '
f'defined! Example: `account.{name}(..., signature="{sig}")`\n'
f'Known signatures: {[entry.signature[len(name):] for entry in self.__hashes[name]]}')

tx_data = ABI.function_call(str(entries[0].signature), *args)
except KeyError as e:
raise e

if caller is None:
caller = self.__default_caller

self._manticore.transaction(caller=caller,
address=self._address,
value=value,
data=tx_data,
gas=gas)
return f

raise AttributeError(f"The contract {self._name} doesn't have {name} function.")
19 changes: 19 additions & 0 deletions tests/eth_general.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,25 @@ def test_create_contract_two_instances(self):
self.assertEqual(len(contracts), len(set(c.address for c in contracts)))
self.assertEqual(len(contracts), len(set(c.name_ for c in contracts)))

def test_contract_create_and_call_underscore_function(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are a hero for including these tests!! :D

source_code = 'contract A { function _f(uint x) returns (uint) { return x + 0x1234; } }'

owner = self.mevm.create_account()
contract = self.mevm.solidity_create_contract(source_code, owner=owner, args=[])

contract._f(123)

def test_contract_create_and_access_non_existing_function(self):
source_code = 'contract A {}'

owner = self.mevm.create_account()
contract = self.mevm.solidity_create_contract(source_code, owner=owner, args=[])

with self.assertRaises(AttributeError) as e:
_ = contract.xyz

self.assertEqual(str(e.exception), "The contract contract0 doesn't have xyz function.")

def test_invalid_function_signature(self):
source_code = '''
contract Test{
Expand Down