Rooba
/
oss-fuzz
mirror of https://github.com/google/oss-fuzz.git
1
1
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[cifuzz][NFC] Handle TODOs (#5104) 

Handle some TODOs
1. Get rid of multiple return values and replace with a more sensible
return value.
2. Eliminate some useless TODOs.
This commit is contained in:
jonathanmetzman 2021-02-04 07:15:51 -08:00 committed by GitHub
parent a21e218511
commit 21b47a7a22
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 45 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
infra/cifuzz/build_fuzzers.py
View File

@ -30,7 +30,6 @@ import utils
DEFAULT_ENGINE = 'libfuzzer'
DEFAULT_ARCHITECTURE = 'x86_64'
# TODO(metzman): Turn default logging to WARNING when CIFuzz is stable.
logging.basicConfig(
format='%(asctime)s - %(name)s - %(levelname)s - %(message)s',
level=logging.DEBUG)

2
infra/cifuzz/clusterfuzz_deployment.py
View File

@ -158,7 +158,7 @@ def download_url(url, filename, num_attempts=3):
"""
sleep_time = 1
# TODO(metzman): Use retry.wrap here.
# Don't use retry wrapper since we don't want this to raise any exceptions.
for _ in range(num_attempts):
try:
urllib.request.urlretrieve(url, filename)

18
infra/cifuzz/fuzz_target.py
View File

@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
"""A module to handle running a fuzz target for a specified amount of time."""
import collections
import logging
import os
import re
@ -23,7 +24,6 @@ import sys
sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
import utils
# TODO: Turn default logging to WARNING when CIFuzz is stable.
logging.basicConfig(
format='%(asctime)s - %(name)s - %(levelname)s - %(message)s',
level=logging.DEBUG)
@ -42,6 +42,8 @@ COULD_NOT_TEST_ON_RECENT_MESSAGE = (
'target to determine if this code change (pr/commit) introduced crash. '
'Assuming this code change introduced crash.')
FuzzResult = collections.namedtuple('FuzzResult', ['testcase', 'stacktrace'])
class ReproduceError(Exception):
"""Error for when we can't attempt to reproduce a crash."""
@ -81,10 +83,8 @@ class FuzzTarget:
"""Starts the fuzz target run for the length of time specified by duration.
Returns:
(testcase, stacktrace, time in seconds) on crash or
(None, None, time in seconds) on timeout or error.
FuzzResult namedtuple with stacktrace and testcase if applicable.
"""
# TODO(metzman): Change return value to a FuzzResult object.
logging.info('Fuzzer %s, started.', self.target_name)
docker_container = utils.get_container_name()
command = ['docker', 'run', '--rm', '--privileged']
@ -122,23 +122,23 @@ class FuzzTarget:
_, stderr = process.communicate(timeout=self.duration + BUFFER_TIME)
except subprocess.TimeoutExpired:
logging.error('Fuzzer %s timed out, ending fuzzing.', self.target_name)
return None, None
return FuzzResult(None, None)
# Libfuzzer timeout was reached.
if not process.returncode:
logging.info('Fuzzer %s finished with no crashes discovered.',
self.target_name)
return None, None
return FuzzResult(None, None)
# Crash was discovered.
logging.info('Fuzzer %s, ended before timeout.', self.target_name)
testcase = self.get_testcase(stderr)
if not testcase:
logging.error(b'No testcase found in stacktrace: %s.', stderr)
return None, None
return FuzzResult(None, None)
if self.is_crash_reportable(testcase):
return testcase, stderr
return None, None
return FuzzResult(testcase, stderr)
return FuzzResult(None, None)
def is_reproducible(self, testcase, target_path):
"""Checks if the testcase reproduces.

33
infra/cifuzz/run_fuzzers.py
View File

@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
"""Module for running fuzzers."""
import enum
import logging
import os
import shutil
@ -28,6 +29,13 @@ sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
import utils
class RunFuzzersResult(enum.Enum):
"""Enum result from running fuzzers."""
ERROR = 0
BUG_FOUND = 1
NO_BUG_FOUND = 2
class BaseFuzzTargetRunner:
"""Base class for fuzzer runners."""
@ -120,28 +128,29 @@ class BaseFuzzTargetRunner:
target = self.create_fuzz_target_obj(target_path, run_seconds)
start_time = time.time()
testcase, stacktrace = self.run_fuzz_target(target)
result = self.run_fuzz_target(target)
# It's OK if this goes negative since we take max when determining
# run_seconds.
fuzz_seconds -= time.time() - start_time
fuzzers_left_to_run -= 1
if not testcase or not stacktrace:
if not result.testcase or not result.stacktrace:
logging.info('Fuzzer %s finished running without crashes.',
target.target_name)
continue
# We found a bug in the fuzz target.
utils.binary_print(b'Fuzzer: %s. Detected bug:\n%s' %
(target.target_name.encode(), stacktrace))
(target.target_name.encode(), result.stacktrace))
# TODO(metzman): Do this with filestore.
testcase_artifact = self.get_fuzz_target_artifact(target, 'testcase')
shutil.move(testcase, testcase_artifact)
bug_summary_artifact = self.get_fuzz_target_artifact(
testcase_artifact_path = self.get_fuzz_target_artifact(target, 'testcase')
shutil.move(result.testcase, testcase_artifact_path)
bug_summary_artifact_path = self.get_fuzz_target_artifact(
target, 'bug-summary.txt')
stack_parser.parse_fuzzer_output(stacktrace, bug_summary_artifact)
stack_parser.parse_fuzzer_output(result.stacktrace,
bug_summary_artifact_path)
bug_found = True
if self.quit_on_bug_found:
@ -183,19 +192,17 @@ def run_fuzzers(config): # pylint: disable=too-many-locals
config: A RunFuzzTargetsConfig.
Returns:
(True if no (internal) errors fuzzing, True if bug found fuzzing).
A RunFuzzersResult enum value indicating what happened during fuzzing.
"""
fuzz_target_runner = get_fuzz_target_runner(config)
# TODO(metzman): Multiple return bools is confusing. Change to one enum
# return value.
if not fuzz_target_runner.initialize():
# We didn't fuzz at all because of internal (CIFuzz) errors. And we didn't
# find any bugs.
return False, False
return RunFuzzersResult.ERROR
if not fuzz_target_runner.run_fuzz_targets():
# We fuzzed successfully, but didn't find any bugs (in the fuzz target).
return True, False
return RunFuzzersResult.NO_BUG_FOUND
# We fuzzed successfully and found bug(s) in the fuzz targets.
return True, True
return RunFuzzersResult.BUG_FOUND

7
infra/cifuzz/run_fuzzers_entrypoint.py
View File

@ -21,7 +21,6 @@ import run_fuzzers
# pylint: disable=c-extension-no-member
# pylint gets confused because of the relative import of cifuzz.
# TODO: Turn default logging to INFO when CIFuzz is stable.
logging.basicConfig(
format='%(asctime)s - %(name)s - %(levelname)s - %(message)s',
level=logging.DEBUG)
@ -64,12 +63,12 @@ def main():
return returncode
# Run the specified project's fuzzers from the build.
run_status, bug_found = run_fuzzers.run_fuzzers(config)
if not run_status:
result = run_fuzzers.run_fuzzers(config)
if result == run_fuzzers.RunFuzzersResult.ERROR:
logging.error('Error occurred while running in workspace %s.',
config.workspace)
return returncode
if bug_found:
if result == run_fuzzers.RunFuzzersResult.BUG_FOUND:
logging.info('Bug found.')
if not config.dry_run:
# Return 2 when a bug was found by a fuzzer causing the CI to fail.

26
infra/cifuzz/run_fuzzers_test.py
View File

@ -23,6 +23,7 @@ import parameterized
from pyfakefs import fake_filesystem_unittest
import config_utils
import fuzz_target
import run_fuzzers
# pylint: disable=wrong-import-position
@ -79,9 +80,8 @@ class RunFuzzerIntegrationTestMixin: # pylint: disable=too-few-public-methods,i
workspace=fuzzer_dir_copy,
project_name='curl',
sanitizer=sanitizer)
run_success, bug_found = run_fuzzers.run_fuzzers(config)
self.assertTrue(run_success)
self.assertFalse(bug_found)
result = run_fuzzers.run_fuzzers(config)
self.assertEqual(result, run_fuzzers.RunFuzzersResult.NO_BUG_FOUND)
class RunMemoryFuzzerIntegrationTest(RunFuzzerIntegrationTestMixin,
@ -257,7 +257,8 @@ class CiFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
testcase = os.path.join(workspace, 'testcase')
self.fs.create_file(testcase)
stacktrace = b'stacktrace'
mocked_run_fuzz_target.return_value = (testcase, stacktrace)
mocked_run_fuzz_target.return_value = fuzz_target.FuzzResult(
testcase, stacktrace)
magic_mock = mock.MagicMock()
magic_mock.target_name = 'target1'
mocked_create_fuzz_target_obj.return_value = magic_mock
@ -304,7 +305,7 @@ class BatchFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
testcase = testcase2
assert call_count != 2
call_count += 1
return testcase, stacktrace
return fuzz_target.FuzzResult(testcase, stacktrace)
mocked_run_fuzz_target.side_effect = mock_run_fuzz_target
magic_mock = mock.MagicMock()
@ -336,9 +337,8 @@ class RunAddressFuzzersIntegrationTest(RunFuzzerIntegrationTestMixin,
config = _create_config(fuzz_seconds=FUZZ_SECONDS,
workspace=workspace,
project_name=EXAMPLE_PROJECT)
run_success, bug_found = run_fuzzers.run_fuzzers(config)
self.assertTrue(run_success)
self.assertTrue(bug_found)
result = run_fuzzers.run_fuzzers(config)
self.assertEqual(result, run_fuzzers.RunFuzzersResult.BUG_FOUND)
build_dir = os.path.join(workspace, 'out', self.BUILD_DIR_NAME)
self.assertNotEqual(0, len(os.listdir(build_dir)))
@ -357,12 +357,11 @@ class RunAddressFuzzersIntegrationTest(RunFuzzerIntegrationTestMixin,
config = _create_config(fuzz_seconds=FUZZ_SECONDS,
workspace=TEST_FILES_PATH,
project_name=EXAMPLE_PROJECT)
run_success, bug_found = run_fuzzers.run_fuzzers(config)
result = run_fuzzers.run_fuzzers(config)
self.assertEqual(result, run_fuzzers.RunFuzzersResult.NO_BUG_FOUND)
build_dir = os.path.join(TEST_FILES_PATH, 'out', self.BUILD_DIR_NAME)
self.assertTrue(os.path.exists(build_dir))
self.assertNotEqual(0, len(os.listdir(build_dir)))
self.assertTrue(run_success)
self.assertFalse(bug_found)
def test_invalid_build(self):
"""Tests run_fuzzers with an invalid ASAN build."""
@ -372,9 +371,8 @@ class RunAddressFuzzersIntegrationTest(RunFuzzerIntegrationTestMixin,
config = _create_config(fuzz_seconds=FUZZ_SECONDS,
workspace=tmp_dir,
project_name=EXAMPLE_PROJECT)
run_success, bug_found = run_fuzzers.run_fuzzers(config)
self.assertFalse(run_success)
self.assertFalse(bug_found)
result = run_fuzzers.run_fuzzers(config)
self.assertEqual(result, run_fuzzers.RunFuzzersResult.ERROR)
if __name__ == '__main__':