From 72eed3148f6773f1bb073f8d74d76d73ab4f987b Mon Sep 17 00:00:00 2001 From: Alan Jowett Date: Thu, 17 Oct 2024 13:13:32 -0700 Subject: [PATCH] Refine rejection criteria Signed-off-by: Alan Jowett --- .gitignore | 118 ----------------------------------- libfuzzer/libfuzz_harness.cc | 21 ++++++- vm/ubpf_vm.c | 24 ++++--- 3 files changed, 35 insertions(+), 128 deletions(-) delete mode 100644 .gitignore diff --git a/.gitignore b/.gitignore deleted file mode 100644 index 672fbe7f2..000000000 --- a/.gitignore +++ /dev/null @@ -1,118 +0,0 @@ -# Byte-compiled / optimized / DLL files -__pycache__/ -*.py[cod] -*$py.class - -# C extensions -*.so - -# Distribution / packaging -.Python -build/ -develop-eggs/ -dist/ -downloads/ -eggs/ -.eggs/ -lib/ -lib64/ -parts/ -sdist/ -var/ -wheels/ -share/python-wheels/ -*.egg-info/ -.installed.cfg -*.egg -MANIFEST - -# PyInstaller -# Usually these files are written by a python script from a template -# before PyInstaller builds the exe, so as to inject date/other infos into it. -*.manifest -*.spec - -# Installer logs -pip-log.txt -pip-delete-this-directory.txt - -# Unit test / coverage reports -htmlcov/ -.tox/ -.nox/ -.coverage -.coverage.* -.cache -nosetests.xml -coverage.xml -*.cover -.hypothesis/ -.pytest_cache/ - -# Translations -*.mo -*.pot - -# Django stuff: -*.log -local_settings.py -db.sqlite3 - -# Flask stuff: -instance/ -.webassets-cache - -# Scrapy stuff: -.scrapy - -# Sphinx documentation -docs/_build/ - -# PyBuilder -target/ - -# Jupyter Notebook -.ipynb_checkpoints - -# IPython -profile_default/ -ipython_config.py - -# pyenv -.python-version - -# celery beat schedule file -celerybeat-schedule - -# SageMath parsed files -*.sage.py - -# Environments -.env -.venv -env/ -venv/ -ENV/ -env.bak/ -venv.bak/ - -# Spyder project settings -.spyderproject -.spyproject - -# Rope project settings -.ropeproject - -# mkdocs documentation -/site - -# mypy -.mypy_cache/ -.dmypy.json -dmypy.json - -# Pyre type checker -.pyre/ - -.vscode/ -.vs/ \ No newline at end of file diff --git a/libfuzzer/libfuzz_harness.cc b/libfuzzer/libfuzz_harness.cc index 0f32d708e..21140e25f 100644 --- a/libfuzzer/libfuzz_harness.cc +++ b/libfuzzer/libfuzz_harness.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -277,7 +278,7 @@ int capture_printf(FILE* stream, const char* format, ...) * @retval false The program might be unsafe to run. Note: The verifier is conservative and may reject safe programs. */ bool -verify_bpf_byte_code(const std::vector& program_code) +verify_bpf_byte_code(const std::vector& program_code, bool get_report = false) try { std::ostringstream error; auto instruction_array = reinterpret_cast(program_code.data()); @@ -307,7 +308,7 @@ try { // Enable termination checking and pre-invariant storage. options.check_termination = true; options.assume_assertions = true; - options.print_invariants = true; + options.print_invariants = get_report; #if defined(UBPF_ENABLE_LIBFUZZER_CONSTRAINT_CHECK) options.store_pre_invariants = true; #endif @@ -571,6 +572,11 @@ bool bounds_check(void* context, uint64_t addr, uint64_t size) return false; } +const std::set g_error_message_to_ignore{ + "Call to local function at pc [0-9]+ is not from a call instruction.", + "Instruction limit exceeded", +}; + /** * @brief Invoke the ubpf interpreter with the given program code and input memory. * @@ -602,6 +608,15 @@ call_ubpf_interpreter( // Execute the program using the input memory. if (ubpf_exec_ex(vm.get(), &context, sizeof(context), &interpreter_result, ubpf_stack.data(), ubpf_stack.size()) != 0) { + // Check if the error is being suppressed by one of the known error messages regex. + for (const auto& error_message : g_error_message_to_ignore) { + if (std::regex_search(g_error_message, std::regex(error_message))) { + return false; + } + } + + // Rerun the verifier to full report. + verify_bpf_byte_code(program_code, true); std::cerr << g_verifier_report << std::endl; throw std::runtime_error("Failed to execute program with error: " + g_error_message); } @@ -733,7 +748,7 @@ LLVMFuzzerTestOneInput(const uint8_t* data, std::size_t size) return -1; } - if (!verify_bpf_byte_code(program)) { + if (!verify_bpf_byte_code(program, false)) { // The program failed verification. return 0; } diff --git a/vm/ubpf_vm.c b/vm/ubpf_vm.c index b05546313..523b68723 100644 --- a/vm/ubpf_vm.c +++ b/vm/ubpf_vm.c @@ -470,7 +470,7 @@ ubpf_check_shadow_stack( * @return false - The registers are not initialized - an error message has been printed. */ static inline bool -ubpf_validate_shadow_register(const struct ubpf_vm* vm, uint16_t* shadow_registers, struct ebpf_inst inst) +ubpf_validate_shadow_register(const struct ubpf_vm* vm, uint32_t pc, uint16_t* shadow_registers, struct ebpf_inst inst) { if (!vm->undefined_behavior_check_enabled) { return true; @@ -497,7 +497,7 @@ ubpf_validate_shadow_register(const struct ubpf_vm* vm, uint16_t* shadow_registe // Store indirect instructions require both the source and destination registers to be initialized. case EBPF_CLS_STX: dst_register_required = true; - src_register_required = true; + src_register_required = (inst.dst != BPF_REG_10); // Writing uninitialized memory to the stack is allowed. break; case EBPF_CLS_ALU: case EBPF_CLS_ALU64: @@ -521,7 +521,16 @@ ubpf_validate_shadow_register(const struct ubpf_vm* vm, uint16_t* shadow_registe dst_register_required = true; break; case 0xb0: // EBPF_OP_MOV - // Destination register is initialized. + // Special case: + // If the source register is not initialized, the destination register is not initialized. + // If the source register is initialized, the destination register is initialized. + src_register_required = false; + // If source register is not initialized, the destination register is not initialized. + if (!((*shadow_registers) & REGISTER_TO_SHADOW_MASK(inst.src))) { + *shadow_registers &= ~REGISTER_TO_SHADOW_MASK(inst.dst); + } else { + dst_register_initialized = true; + } break; } break; @@ -553,12 +562,12 @@ ubpf_validate_shadow_register(const struct ubpf_vm* vm, uint16_t* shadow_registe } if (src_register_required && !(*shadow_registers & REGISTER_TO_SHADOW_MASK(inst.src))) { - vm->error_printf(stderr, "Error: Source register r%d is not initialized.\n", inst.src); + vm->error_printf(stderr, "Error: %d: Source register r%d is not initialized.\n", pc, inst.src); return false; } if (dst_register_required && !(*shadow_registers & REGISTER_TO_SHADOW_MASK(inst.dst))) { - vm->error_printf(stderr, "Error: Destination register r%d is not initialized.\n", inst.dst); + vm->error_printf(stderr, "Error: %d Destination register r%d is not initialized.\n", pc, inst.dst); return false; } @@ -582,7 +591,7 @@ ubpf_validate_shadow_register(const struct ubpf_vm* vm, uint16_t* shadow_registe if (inst.opcode == EBPF_OP_EXIT) { if (!(*shadow_registers & REGISTER_TO_SHADOW_MASK(0))) { - vm->error_printf(stderr, "Error: Return value register r0 is not initialized.\n"); + vm->error_printf(stderr, "Error: %d: Return value register r0 is not initialized.\n", pc); return false; } // Mark r1-r5 as uninitialized. @@ -658,6 +667,7 @@ ubpf_exec_ex( } if (vm->instruction_limit && instruction_limit-- <= 0) { return_value = -1; + vm->error_printf(stderr, "Error: Instruction limit exceeded.\n"); goto cleanup; } @@ -677,7 +687,7 @@ ubpf_exec_ex( struct ebpf_inst inst = ubpf_fetch_instruction(vm, pc++); - if (!ubpf_validate_shadow_register(vm, &shadow_registers, inst)) { + if (!ubpf_validate_shadow_register(vm, cur_pc, &shadow_registers, inst)) { return_value = -1; goto cleanup; }