From 21ea9d6e3dcd85d40efd8943c3fa0e75eae09fd2 Mon Sep 17 00:00:00 2001 From: Abhishek Arya Date: Wed, 15 Jul 2020 13:00:57 -0700 Subject: [PATCH] Remove sanitizer validation checks in CIFuzz, (#4131) Sanitizer validation is not needed and it is hacky code with no proper yaml parsing. sanitizer attribute is properly documented. Fixes #3996. --- infra/cifuzz/cifuzz.py | 44 +----------------- infra/cifuzz/cifuzz_test.py | 90 +++++-------------------------------- 2 files changed, 13 insertions(+), 121 deletions(-) diff --git a/infra/cifuzz/cifuzz.py b/infra/cifuzz/cifuzz.py index 9f50b2f2c..1d3d0e2f7 100644 --- a/infra/cifuzz/cifuzz.py +++ b/infra/cifuzz/cifuzz.py @@ -107,10 +107,7 @@ def build_fuzzers(project_name, logging.error('Invalid workspace: %s.', workspace) return False - # Check that sanitizer is valid. - if not is_project_sanitizer(sanitizer, project_name): - raise ValueError("{0} is not a valid sanitizer for project {1}".format( - sanitizer, project_name)) + logging.info("Using %s sanitizer.", sanitizer) git_workspace = os.path.join(workspace, 'storage') os.makedirs(git_workspace, exist_ok=True) @@ -200,11 +197,7 @@ def run_fuzzers(fuzz_seconds, workspace, project_name, sanitizer='address'): logging.error('Invalid workspace: %s.', workspace) return False, False - # Check that sanitizer is valid. - if not is_project_sanitizer(sanitizer, project_name): - raise ValueError("{0} is not a valid sanitizer for project {1}".format( - sanitizer, project_name)) - logging.info("Using %s as sanitizer.", sanitizer) + logging.info("Using %s sanitizer.", sanitizer) out_dir = os.path.join(workspace, 'out') artifacts_dir = os.path.join(out_dir, 'artifacts') @@ -500,36 +493,3 @@ def parse_fuzzer_output(fuzzer_output, out_dir): summary_file_path = os.path.join(out_dir, 'bug_summary.txt') with open(summary_file_path, 'a') as summary_handle: summary_handle.write(summary_str) - - -def is_project_sanitizer(sanitizer, oss_fuzz_project_name): - """Finds all of the sanitizers a project can use for building and running. - - Args: - sanitizer: The desired sanitizer. - oss_fuzz_project_name: The name of the relevant OSS-Fuzz project. - - Returns: - True if project can use sanitizer. - """ - project_yaml = os.path.join(helper.OSS_FUZZ_DIR, 'projects', - oss_fuzz_project_name, 'project.yaml') - if not os.path.isfile(project_yaml): - logging.error('project.yaml for project %s could not be found.', - oss_fuzz_project_name) - return False - - # Simple parse to prevent adding pyYAML dependency. - with open(project_yaml, 'r') as file_handle: - file_data = file_handle.read() - - # TODO(metzman): Replace this with proper handling of project.yaml files. - if 'sanitizers:' not in file_data: - logging.info('No sanitizers defined in project.yaml. ' - 'Only allowing address and undefined') - - # List is from: - # https://google.github.io/oss-fuzz/getting-started/new-project-guide/#sanitizers - return sanitizer in ('address', 'undefined') - - return sanitizer in file_data diff --git a/infra/cifuzz/cifuzz_test.py b/infra/cifuzz/cifuzz_test.py index 6cf6d4270..abc24469d 100644 --- a/infra/cifuzz/cifuzz_test.py +++ b/infra/cifuzz/cifuzz_test.py @@ -24,8 +24,6 @@ import tempfile import unittest from unittest import mock -from pyfakefs import fake_filesystem_unittest - # pylint: disable=wrong-import-position INFRA_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.append(INFRA_DIR) @@ -108,13 +106,13 @@ class BuildFuzzersIntegrationTest(unittest.TestCase): def test_invalid_project_name(self): """Test building fuzzers with invalid project name.""" - with tempfile.TemporaryDirectory() as tmp_dir, self.assertRaises( - ValueError): - cifuzz.build_fuzzers( - 'not_a_valid_project', - 'oss-fuzz', - tmp_dir, - commit_sha='0b95fe1039ed7c38fea1f97078316bfc1030c523') + with tempfile.TemporaryDirectory() as tmp_dir: + self.assertFalse( + cifuzz.build_fuzzers( + 'not_a_valid_project', + 'oss-fuzz', + tmp_dir, + commit_sha='0b95fe1039ed7c38fea1f97078316bfc1030c523')) def test_invalid_repo_name(self): """Test building fuzzers with invalid repo name.""" @@ -223,10 +221,7 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): # OSS-Fuzz build. with mock.patch.object(fuzz_target.FuzzTarget, 'is_reproducible', - side_effect=[True, False]), mock.patch.object( - cifuzz, - 'is_project_sanitizer', - return_value=True): + side_effect=[True, False]): run_success, bug_found = cifuzz.run_fuzzers(10, TEST_FILES_PATH, EXAMPLE_PROJECT) build_dir = os.path.join(TEST_FILES_PATH, 'out', 'oss_fuzz_latest') @@ -239,10 +234,7 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): """Test run_fuzzers with a bug found in OSS-Fuzz before.""" with mock.patch.object(fuzz_target.FuzzTarget, 'is_reproducible', - side_effect=[True, True]), mock.patch.object( - cifuzz, - 'is_project_sanitizer', - return_value=True): + side_effect=[True, True]): run_success, bug_found = cifuzz.run_fuzzers(10, TEST_FILES_PATH, EXAMPLE_PROJECT) build_dir = os.path.join(TEST_FILES_PATH, 'out', 'oss_fuzz_latest') @@ -253,8 +245,7 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): def test_invalid_build(self): """Test run_fuzzers with an invalid build.""" - with tempfile.TemporaryDirectory() as tmp_dir, unittest.mock.patch.object( - cifuzz, 'is_project_sanitizer', return_value=True): + with tempfile.TemporaryDirectory() as tmp_dir: out_path = os.path.join(tmp_dir, 'out') os.mkdir(out_path) run_success, bug_found = cifuzz.run_fuzzers(10, tmp_dir, EXAMPLE_PROJECT) @@ -263,8 +254,7 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase): def test_invalid_fuzz_seconds(self): """Tests run_fuzzers with an invalid fuzz seconds.""" - with tempfile.TemporaryDirectory() as tmp_dir, unittest.mock.patch.object( - cifuzz, 'is_project_sanitizer', return_value=True): + with tempfile.TemporaryDirectory() as tmp_dir: out_path = os.path.join(tmp_dir, 'out') os.mkdir(out_path) run_success, bug_found = cifuzz.run_fuzzers(0, tmp_dir, EXAMPLE_PROJECT) @@ -507,64 +497,6 @@ class KeepAffectedFuzzersUnitTest(unittest.TestCase): self.assertEqual(2, len(os.listdir(tmp_dir))) -class IsProjectSanitizerUnitTest(fake_filesystem_unittest.TestCase): - """Class to test the is_project_sanitizer function in the cifuzz module. - Note: This test relies on the curl project being an OSS-Fuzz project. - """ - - def setUp(self): - self.setUpPyfakefs() - self.fake_project = 'fake_project' - self.project_yaml = os.path.join(OSS_FUZZ_DIR, 'projects', - self.fake_project, 'project.yaml') - - def test_valid_project_curl(self): - """Test if sanitizers can be detected from project.yaml""" - self.fs.add_real_directory(OSS_FUZZ_DIR) - self.assertTrue(cifuzz.is_project_sanitizer('memory', 'curl')) - self.assertTrue(cifuzz.is_project_sanitizer('address', 'curl')) - self.assertTrue(cifuzz.is_project_sanitizer('undefined', 'curl')) - self.assertFalse(cifuzz.is_project_sanitizer('not-a-san', 'curl')) - - def test_valid_project_example(self): - """Test if sanitizers can be detected from project.yaml""" - self.fs.add_real_directory(OSS_FUZZ_DIR) - self.assertFalse(cifuzz.is_project_sanitizer('memory', 'example')) - self.assertTrue(cifuzz.is_project_sanitizer('address', 'example')) - self.assertTrue(cifuzz.is_project_sanitizer('undefined', 'example')) - self.assertFalse(cifuzz.is_project_sanitizer('not-a-san', 'example')) - - def test_invalid_project(self): - """Tests that invalid projects return false.""" - self.fs.add_real_directory(OSS_FUZZ_DIR) - self.assertFalse(cifuzz.is_project_sanitizer('memory', 'notaproj')) - self.assertFalse(cifuzz.is_project_sanitizer('address', 'notaproj')) - self.assertFalse(cifuzz.is_project_sanitizer('undefined', 'notaproj')) - - def test_no_specified_sanitizers(self): - """Tests is_project_sanitizer returns True for any fuzzer if non are - specified.""" - contents = 'homepage: "https://my-api.example.com' - self.fs.create_file(self.project_yaml, contents=contents) - self.assertTrue(cifuzz.is_project_sanitizer('address', self.fake_project)) - self.assertTrue(cifuzz.is_project_sanitizer('undefined', self.fake_project)) - self.assertFalse(cifuzz.is_project_sanitizer('memory', self.fake_project)) - self.assertFalse(cifuzz.is_project_sanitizer('fake', self.fake_project)) - - def test_experimental_sanitizer(self): - """Tests that experimental sanitizers are handled properly.""" - contents = ('homepage: "https://my-api.example.com\n' - 'sanitizers:\n' - 'memory:\n' - 'experimental: True\n' - '- address') - self.fs.create_file(self.project_yaml, contents=contents) - self.assertTrue(cifuzz.is_project_sanitizer('address', self.fake_project)) - self.assertFalse(cifuzz.is_project_sanitizer('undefined', - self.fake_project)) - self.assertTrue(cifuzz.is_project_sanitizer('memory', self.fake_project)) - - @unittest.skip('Test is too long to be run with presubmit.') class BuildSantizerIntegrationTest(unittest.TestCase): """Integration tests for the build_fuzzers function in the cifuzz module.