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.
This commit is contained in:
Abhishek Arya 2020-07-15 13:00:57 -07:00 committed by GitHub
parent a14eee4c42
commit 21ea9d6e3d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 13 additions and 121 deletions

View File

@ -107,10 +107,7 @@ def build_fuzzers(project_name,
logging.error('Invalid workspace: %s.', workspace) logging.error('Invalid workspace: %s.', workspace)
return False return False
# Check that sanitizer is valid. logging.info("Using %s sanitizer.", sanitizer)
if not is_project_sanitizer(sanitizer, project_name):
raise ValueError("{0} is not a valid sanitizer for project {1}".format(
sanitizer, project_name))
git_workspace = os.path.join(workspace, 'storage') git_workspace = os.path.join(workspace, 'storage')
os.makedirs(git_workspace, exist_ok=True) 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) logging.error('Invalid workspace: %s.', workspace)
return False, False return False, False
# Check that sanitizer is valid. logging.info("Using %s sanitizer.", sanitizer)
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)
out_dir = os.path.join(workspace, 'out') out_dir = os.path.join(workspace, 'out')
artifacts_dir = os.path.join(out_dir, 'artifacts') 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') summary_file_path = os.path.join(out_dir, 'bug_summary.txt')
with open(summary_file_path, 'a') as summary_handle: with open(summary_file_path, 'a') as summary_handle:
summary_handle.write(summary_str) 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

View File

@ -24,8 +24,6 @@ import tempfile
import unittest import unittest
from unittest import mock from unittest import mock
from pyfakefs import fake_filesystem_unittest
# pylint: disable=wrong-import-position # pylint: disable=wrong-import-position
INFRA_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) INFRA_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
sys.path.append(INFRA_DIR) sys.path.append(INFRA_DIR)
@ -108,13 +106,13 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
def test_invalid_project_name(self): def test_invalid_project_name(self):
"""Test building fuzzers with invalid project name.""" """Test building fuzzers with invalid project name."""
with tempfile.TemporaryDirectory() as tmp_dir, self.assertRaises( with tempfile.TemporaryDirectory() as tmp_dir:
ValueError): self.assertFalse(
cifuzz.build_fuzzers( cifuzz.build_fuzzers(
'not_a_valid_project', 'not_a_valid_project',
'oss-fuzz', 'oss-fuzz',
tmp_dir, tmp_dir,
commit_sha='0b95fe1039ed7c38fea1f97078316bfc1030c523') commit_sha='0b95fe1039ed7c38fea1f97078316bfc1030c523'))
def test_invalid_repo_name(self): def test_invalid_repo_name(self):
"""Test building fuzzers with invalid repo name.""" """Test building fuzzers with invalid repo name."""
@ -223,10 +221,7 @@ class RunAddressFuzzersIntegrationTest(unittest.TestCase):
# OSS-Fuzz build. # OSS-Fuzz build.
with mock.patch.object(fuzz_target.FuzzTarget, with mock.patch.object(fuzz_target.FuzzTarget,
'is_reproducible', 'is_reproducible',
side_effect=[True, False]), mock.patch.object( side_effect=[True, False]):
cifuzz,
'is_project_sanitizer',
return_value=True):
run_success, bug_found = cifuzz.run_fuzzers(10, TEST_FILES_PATH, run_success, bug_found = cifuzz.run_fuzzers(10, TEST_FILES_PATH,
EXAMPLE_PROJECT) EXAMPLE_PROJECT)
build_dir = os.path.join(TEST_FILES_PATH, 'out', 'oss_fuzz_latest') 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.""" """Test run_fuzzers with a bug found in OSS-Fuzz before."""
with mock.patch.object(fuzz_target.FuzzTarget, with mock.patch.object(fuzz_target.FuzzTarget,
'is_reproducible', 'is_reproducible',
side_effect=[True, True]), mock.patch.object( side_effect=[True, True]):
cifuzz,
'is_project_sanitizer',
return_value=True):
run_success, bug_found = cifuzz.run_fuzzers(10, TEST_FILES_PATH, run_success, bug_found = cifuzz.run_fuzzers(10, TEST_FILES_PATH,
EXAMPLE_PROJECT) EXAMPLE_PROJECT)
build_dir = os.path.join(TEST_FILES_PATH, 'out', 'oss_fuzz_latest') 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): def test_invalid_build(self):
"""Test run_fuzzers with an invalid build.""" """Test run_fuzzers with an invalid build."""
with tempfile.TemporaryDirectory() as tmp_dir, unittest.mock.patch.object( with tempfile.TemporaryDirectory() as tmp_dir:
cifuzz, 'is_project_sanitizer', return_value=True):
out_path = os.path.join(tmp_dir, 'out') out_path = os.path.join(tmp_dir, 'out')
os.mkdir(out_path) os.mkdir(out_path)
run_success, bug_found = cifuzz.run_fuzzers(10, tmp_dir, EXAMPLE_PROJECT) 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): def test_invalid_fuzz_seconds(self):
"""Tests run_fuzzers with an invalid fuzz seconds.""" """Tests run_fuzzers with an invalid fuzz seconds."""
with tempfile.TemporaryDirectory() as tmp_dir, unittest.mock.patch.object( with tempfile.TemporaryDirectory() as tmp_dir:
cifuzz, 'is_project_sanitizer', return_value=True):
out_path = os.path.join(tmp_dir, 'out') out_path = os.path.join(tmp_dir, 'out')
os.mkdir(out_path) os.mkdir(out_path)
run_success, bug_found = cifuzz.run_fuzzers(0, tmp_dir, EXAMPLE_PROJECT) 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))) 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.') @unittest.skip('Test is too long to be run with presubmit.')
class BuildSantizerIntegrationTest(unittest.TestCase): class BuildSantizerIntegrationTest(unittest.TestCase):
"""Integration tests for the build_fuzzers function in the cifuzz module. """Integration tests for the build_fuzzers function in the cifuzz module.