Skip to content

Commit

Permalink
Refine rejection criteria
Browse files Browse the repository at this point in the history
Signed-off-by: Alan Jowett <[email protected]>
  • Loading branch information
Alan Jowett committed Oct 17, 2024
1 parent f7df350 commit 72eed31
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 128 deletions.
118 changes: 0 additions & 118 deletions .gitignore

This file was deleted.

21 changes: 18 additions & 3 deletions libfuzzer/libfuzz_harness.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <iostream>
#include <memory>
#include <vector>
#include <set>
#include <string>
#include <sstream>

Expand Down Expand Up @@ -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<uint8_t>& program_code)
verify_bpf_byte_code(const std::vector<uint8_t>& program_code, bool get_report = false)
try {
std::ostringstream error;
auto instruction_array = reinterpret_cast<const ebpf_inst*>(program_code.data());
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -571,6 +572,11 @@ bool bounds_check(void* context, uint64_t addr, uint64_t size)
return false;
}

const std::set<std::string> 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.
*
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
Expand Down
24 changes: 17 additions & 7 deletions vm/ubpf_vm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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:
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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.
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}
Expand Down

0 comments on commit 72eed31

Please sign in to comment.