diff --git a/utils/slither_format/slither_format.py b/utils/slither_format/slither_format.py index f065cee47..86bdf0660 100644 --- a/utils/slither_format/slither_format.py +++ b/utils/slither_format/slither_format.py @@ -41,23 +41,47 @@ def slither_format(args, slither): results.extend(detector_results) number_of_slither_results = get_number_of_slither_results(detector_results) apply_detector_results(slither, patches, detector_results) - sort_patches(patches) + sort_and_flag_overlapping_patches(patches) + prune_overlapping_patches(args, patches) if args.verbose_json: print_patches_json(number_of_slither_results, patches) if args.verbose_test: print_patches(number_of_slither_results, patches) apply_patches(slither, patches) -def sort_patches(patches): +def sort_and_flag_overlapping_patches(patches): for file in patches: n = len(patches[file]) for i in range(n): for j in range (0,n-i-1): - if int(patches[file][j]['start']) >= int(patches[file][j+1]['end']): + # Sort check + if int(patches[file][j]['start']) > int(patches[file][j+1]['start']): temp = patches[file][j+1] patches[file][j+1] = patches[file][j] patches[file][j] = temp + # Overlap check + if (int(patches[file][j]['start']) >= int(patches[file][j+1]['start']) and + int(patches[file][j]['start']) <= int(patches[file][j+1]['end'])): + patches[file][j]['overlaps'] = "Yes" + patches[file][j+1]['overlaps'] = "Yes" +def is_overlap_patch(args, patch): + if 'overlaps' in patch: + if args.verbose_test: + print("Overlapping patch won't be applied!") + print("xDetector: " + patch['detector']) + print("xOld string: " + patch['old_string'].replace("\n","")) + print("xNew string: " + patch['new_string'].replace("\n","")) + print("xLocation start: " + str(patch['start'])) + print("xLocation end: " + str(patch['end'])) + return True + return False + +def prune_overlapping_patches(args, patches): + for file in patches: + non_overlapping_patches = [patch for patch in patches[file] if not is_overlap_patch(args, patch)] + patches[file] = non_overlapping_patches + def apply_patches(slither, patches): for file in patches: _in_file = file @@ -110,6 +134,8 @@ def print_patches_json(number_of_slither_results, patches): print("\"New string\":" + '"' + patch['new_string'].replace("\n","") + '",') print("\"Location start\":" + '"' + str(patch['start']) + '",') print("\"Location end\":" + '"' + str(patch['end']) + '"') + if 'overlaps' in patch: + print("\"Overlaps\":" + "Yes") print('}',end='') print(']',end='') print('}',end='') diff --git a/utils/slither_format/tests/run_all_tests.py b/utils/slither_format/tests/run_all_tests.py index 935de5408..87ac64d20 100644 --- a/utils/slither_format/tests/run_all_tests.py +++ b/utils/slither_format/tests/run_all_tests.py @@ -14,4 +14,6 @@ p6 = subprocess.Popen(['python3', './slither_format/tests/test_pragma.py']) p6.wait() p7 = subprocess.Popen(['python3', './slither_format/tests/test_solc_version.py']) p7.wait() +p8 = subprocess.Popen(['python3', './slither_format/tests/test_detector_combinations.py']) +p8.wait() diff --git a/utils/slither_format/tests/test_data/detector_combinations.sol b/utils/slither_format/tests/test_data/detector_combinations.sol new file mode 100644 index 000000000..99011adcf --- /dev/null +++ b/utils/slither_format/tests/test_data/detector_combinations.sol @@ -0,0 +1,48 @@ +pragma solidity ^0.4.24; + +contract A { + + /* constant state variable naming - bad */ + /* unused state variable - bad */ + /* Overlapping detectors - so neither will be applied for now */ + uint max_tx = 100; + + /* state variable declaration naming convention - bad */ + uint SV_count = 0; + + modifier mod (uint c) { + require (c > 100); + _; + } + + /* parameter declaration naming convention - bad */ + function foo(uint Count) { + /* parameter use naming convention- bad */ + /* state variable use naming convention - bad */ + SV_count = Count; + } + + /* implicitly public, can be made external - bad */ + /* parameter declarations naming convention - bad */ + function foobar(uint Count, uint Number) returns (uint) { + /* parameter use naming convention - bad */ + foo (Number); + /* parameter use naming convention - bad */ + return (Count+Number); + } + + /* explicitly public, can be made external - bad */ + /* view but modifies state - bad */ + /* parameter declarations naming convention - bad */ + /* parameter use passed to modifier naming convention - bad */ + function bar(uint Count) public view mod (Count) returns(uint) { + /* Use of state variable naming convention - bad */ + /* Use of parameter naming convention - bad */ + SV_count += Count; + /* Use of state variable naming convention - bad */ + return (SV_count); + } + +} + + diff --git a/utils/slither_format/tests/test_detector_combinations.py b/utils/slither_format/tests/test_detector_combinations.py new file mode 100644 index 000000000..10e60a082 --- /dev/null +++ b/utils/slither_format/tests/test_detector_combinations.py @@ -0,0 +1,40 @@ +import unittest +import subprocess, os, sys + +class TestDetectorCombinations(unittest.TestCase): + testDataDir = "./slither_format/tests/test_data/" + testDataFile1 = "detector_combinations.sol" + testFilePath1 = testDataDir+testDataFile1 + + def setUp(self): + outFD1 = open(self.testFilePath1+".out","w") + errFD1 = open(self.testFilePath1+".err","w") + p1 = subprocess.Popen(['python3', '-m', 'slither_format','--verbose-test',self.testFilePath1], stdout=outFD1,stderr=errFD1) + p1.wait() + outFD1.close() + errFD1.close() + + def tearDown(self): + p1 = subprocess.Popen(['rm','-f',self.testFilePath1+'.out',self.testFilePath1+'.err',self.testFilePath1+'.format']) + p1.wait() + + def test_detector_combinations(self): + outFD1 = open(self.testFilePath1+".out","r") + outFD1_lines = outFD1.readlines() + outFD1.close() + for i in range(len(outFD1_lines)): + outFD1_lines[i] = outFD1_lines[i].strip() + self.assertTrue(os.path.isfile(self.testFilePath1+".format"),"Patched .format file is not created?!") + self.assertEqual(outFD1_lines.count("Number of Slither results: 11"), 1) + self.assertEqual(outFD1_lines.count("Number of patches: 18"), 1) + self.assertEqual(outFD1_lines.count("Overlapping patch won't be applied!"), 2) + self.assertEqual(outFD1_lines.count("xDetector: unused-state"), 1) + self.assertEqual(outFD1_lines.count("xDetector: constable-states"), 1) + self.assertEqual(outFD1_lines.count("Detector: naming-convention (state variable declaration)"), 2) + self.assertEqual(outFD1_lines.count("Detector: naming-convention (state variable uses)"), 3) + self.assertEqual(outFD1_lines.count("Detector: naming-convention (parameter declaration)"), 4) + self.assertEqual(outFD1_lines.count("Detector: naming-convention (parameter uses)"), 6) + self.assertEqual(outFD1_lines.count("Detector: external-function"), 2) + self.assertEqual(outFD1_lines.count("Detector: constant-function"), 1) +if __name__ == '__main__': + unittest.main()