eth-sri / securify

[DEPRECATED] Security Scanner for Ethereum Smart Contracts
Apache License 2.0
215 stars 50 forks source link

implement basic end to end testing #57

Closed hiqua closed 5 years ago

hiqua commented 5 years ago

Add a Python script to run securify on multiple input and see whether their outputs match the ones previously saved, so that changes can be acknowledged and bugs introduced can be fixed.

ghost commented 5 years ago

DeepCode analyzed this pull request. There are no new issues.

hiqua commented 5 years ago

Ideally we'd add a file mentioning that the examples belong to their original authors.

ritzdorf commented 5 years ago

Possibly stupid question: Do we need to check licenses for the included solidity files? Or at least point out all of their origins?

MatthiasEgli-chainsecurity commented 5 years ago

Yes, ideally as part of the sol files header.

ritzdorf commented 5 years ago

@hiqua do you want to add a note in the README (or somewhere else):

ritzdorf commented 5 years ago

Otherwise it looks good to me. Eventually we can add a better diff if needed.

hiqua commented 5 years ago

We also need to look into the license, SWC for example is under GPL and we can't re-license it under Apache. I will reach out to the team there to ask them to dual-license it, let's see if that is an option...

I'll remove the SWC examples for now.

hiqua commented 5 years ago

License issues should be fixed now.

hiqua commented 5 years ago

Every suggestion so far has been implemented.

ritzdorf commented 5 years ago

I get the following error when running test.py:

java -Xmx4G -jar build/libs/securify-0.1.jar -fs src/test/resources/solidity/end_to_end_testing_quick/pax_flattened.sol -o /tmp/tmpke1ftszr/sec_output.json
Different output in src/test/resources/solidity/end_to_end_testing_quick/pax_flattened.sol:PAXImplementation with pattern LockedEther with conflicts!
Traceback (most recent call last):
  File "test.py", line 146, in <module>
    test(QUICK)
  File "test.py", line 138, in test
    test_securify_analysis(contract_file, json_output, overwrite=overwrite)
  File "test.py", line 124, in test_securify_analysis
    check_all_patterns(curr_json, expc_json, ctr)
  File "test.py", line 67, in check_all_patterns
    check_every_match(curr_res, exp_res, ctr, pat)
  File "test.py", line 53, in check_every_match
    raise_mismatch(e_pat_results, c_pat_results)
  File "test.py", line 40, in raise_mismatch
    raise MismatchError(f'Current: {current}, '
__main__.MismatchError: Current: [66], expected: [45]

Is this expected?

hiqua commented 5 years ago

I get the following error when running test.py: ... Is this expected?

Sorry I forgot to update the output when I added the license headers, but obviously they change the line mappings.

Everything has been fixed, let me know if there are still more things.

ghost commented 5 years ago

DeepCode analyzed this pull request. There are no new issues.