Upload folder using huggingface_hub

REPORT.md

@@ -0,0 +1,179 @@
+# Null Pointer Dereference in ExecuTorch Method::execute_instruction() via Missing Operator.overload (.pte)
+
+**Target:** ExecuTorch 1.3.1 (.pte, huntr Model File Vulnerability program)
+**Severity:** Low-Medium (Denial of Service) — CONDITIONAL on `EXECUTORCH_ENABLE_LOGGING=ON`
+**CWE:** CWE-476 (NULL Pointer Dereference)
+**Component:** `runtime/executor/method.cpp`
+**Authentication Required:** No — requires a victim application to execute an attacker-supplied `.pte` file whose method contains an operator that legitimately fails during execution (a routine, expected occurrence).
+
+## Summary
+
+`Method::execute_instruction()`'s `KernelCall` failure-logging path calls `op->overload()->c_str()` unconditionally when a kernel fails. `Operator.overload` is an optional flatbuffer field — a `.pte` file with an `Operator` entry that has a `name` but no `overload` triggers a null-pointer dereference the moment that operator's kernel returns any error during normal execution (e.g. a routine input-shape validation failure). This is distinct from all prior findings in this investigation: it is the first bug reached through actual kernel **execution** (`Method::execute()`), not just method loading, and its practical reachability is conditional on a specific (but officially documented and widely used) build configuration flag.
+
+## Vulnerability Details
+
+`runtime/executor/method.cpp`'s `Method::execute_instruction()`:
+
+```cpp
+case executorch_flatbuffer::InstructionArguments::KernelCall: {
+  ...
+  chain.kernels_[step_state_.instr_idx](context, args);
+  err = context.failure_state();
+  if (err != Error::Ok) {
+    auto op_index = instruction->instr_args_as_KernelCall()->op_index();
+    ET_UNUSED auto op = serialization_plan_->operators()->Get(op_index);
+    ET_LOG(
+        Error,
+        "KernelCall failed at instruction %" ET_PRIsize_t ":%" ET_PRIsize_t
+        " in operator %s.%s: 0x%x",
+        step_state_.chain_idx,
+        step_state_.instr_idx,
+        op->name()->c_str(),
+        op->overload()->c_str(),   // <-- unguarded, crash site
+        (unsigned int)err);
+    ...
+```
+
+The sibling function `populate_operator_name()` (also in `method.cpp`, used during `Method::init()`'s operator resolution) correctly guards this exact field:
+
+```cpp
+const bool has_overload =
+    op->overload() != nullptr && op->overload()->size() > 0;
+...
+has_overload ? op->overload()->c_str() : "",
+```
+
+`execute_instruction()`'s error-logging path independently re-implements similar message construction but omits this guard.
+
+## An Important Investigative Finding: The Crash Is Masked By Default
+
+During investigation, an initial test of this hypothesis against the default sanitized build configuration (`EXECUTORCH_ENABLE_LOGGING` unset, which defaults to OFF per `CMakeLists.txt`) did **not** crash. Root-cause analysis revealed why: `ET_LOG(...)` expands to `((void)0)` when `ET_LOG_ENABLED=0` — the entire log statement, including the `op->overload()->c_str()` argument expression, is never compiled into the code path at all in that configuration.
+
+This was verified two ways before concluding the bug was real:
+
+1. A standalone minimal reproduction program (`check_null.cpp`, not part of the final PoC package) directly loaded the crafted `.pte`, confirmed `op->overload()` returns `nullptr`, and confirmed calling `->c_str()` on it produces the exact `member call on null pointer of type 'flatbuffers::String'` UBSan error.
+2. The full sanitized library was rebuilt with `-DEXECUTORCH_ENABLE_LOGGING=ON` — a real, non-default, but officially documented CMake option, explicitly set in numerous real upstream build scripts (the XNNPACK delegate tutorial, Vulkan delegate tutorial, Qualcomm backend `build.sh`, Cadence backend build scripts, Arm VGF tutorials, and the Llama example README all set `-DEXECUTORCH_ENABLE_LOGGING=ON`). Against this build, the crash reproduced reliably.
+
+This report is filed with that caveat stated explicitly rather than either hiding it (which would overclaim universal reachability) or omitting the finding (which would underclaim, since `EXECUTORCH_ENABLE_LOGGING=ON` is a genuine, common, documented production configuration).
+
+## Steps to Reproduce
+
+### Environment
+Linux x86-64, ExecuTorch 1.3.1 pristine source, clang-16, CMake, Ninja. No authentication, no host access.
+
+### 1. Build ExecuTorch with sanitizers AND logging enabled
+
+```bash
+cmake -S executorch -B build \
+  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+  -DCMAKE_C_COMPILER=clang-16 -DCMAKE_CXX_COMPILER=clang++-16 \
+  -DEXECUTORCH_ENABLE_PROGRAM_VERIFICATION=ON \
+  -DEXECUTORCH_ENABLE_LOGGING=ON \
+  -DCMAKE_C_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer -fno-sanitize-recover=all" \
+  -DCMAKE_CXX_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer -fno-sanitize-recover=all" \
+  -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined" \
+  -DEXECUTORCH_BUILD_EXTENSION_FLAT_TENSOR=ON -DEXECUTORCH_BUILD_EXTENSION_DATA_LOADER=ON \
+  -DEXECUTORCH_BUILD_TESTS=OFF -DEXECUTORCH_BUILD_PYBIND=OFF -DEXECUTORCH_BUILD_XNNPACK=OFF \
+  -G Ninja
+ninja -C build executorch_core extension_flat_tensor extension_data_loader
+```
+
+### 2. Build the execute-level harness (`poc/harness_execute_fuzzer.cpp`, included in this report)
+
+This harness registers fake, deliberately-failing kernels (including one named `test_op` with no overload suffix, and a fake backend) and calls `Program::load_method()` + `Method::execute()` — exercising real kernel invocation, not just metadata:
+
+```bash
+export ET_PARENT=/path/to/parent-of-executorch
+C10_INC="$ET_SRC/runtime/core/portable_type/c10"
+INCLUDES="-I$ET_PARENT -I$ET_BUILD -I$ET_BUILD/schema/include -I$ET_BUILD/extension/flat_tensor/include -I$ET_BUILD/third-party/flatc_ep/include -I$C10_INC"
+
+clang++-16 -std=c++17 -fsanitize=fuzzer,address,undefined -fno-omit-frame-pointer -fno-sanitize-recover=all \
+  $INCLUDES -DFLATBUFFERS_MAX_ALIGNMENT=1024 -DC10_USING_CUSTOM_GENERATED_MACROS \
+  -c poc/harness_execute_fuzzer.cpp -o harness.o
+
+clang++-16 -fsanitize=fuzzer,address,undefined -o poc_harness harness.o \
+  "$ET_BUILD/extension/data_loader/libextension_data_loader.a" \
+  "$ET_BUILD/libexecutorch_core.a"
+```
+
+### 3. PoC file
+
+`poc/poc_operator_overload_null.pte` (368 bytes, **included in this report — sha256 `f6b855b9d4bb69df0519e9954627703d88b9ef4da7104bbd52a6ba33239c3919`**) is a hand-crafted `.pte` containing:
+- One `Operator` entry named `test_op` with **no** `overload` field
+- One `KernelCall` instruction referencing that operator
+- A registered (in the harness) always-failing kernel named `test_op` that matches this operator without needing an overload suffix
+
+### 4. Trigger the crash
+
+```bash
+export ASAN_OPTIONS="abort_on_error=1:symbolize=0"
+export UBSAN_OPTIONS="halt_on_error=1:print_stacktrace=0"
+./poc_harness -timeout=5 -runs=0 poc/poc_operator_overload_null.pte
+```
+
+### Expected result (secure behavior)
+`Method::execute()` should return the kernel's error cleanly (and, if logging, produce a valid log message omitting the missing overload) rather than crashing while constructing the log message.
+
+### Actual result — verified against the pristine, unmodified ExecuTorch 1.3.1 source (with `EXECUTORCH_ENABLE_LOGGING=ON`)
+
+```
+Running: poc/poc_operator_overload_null.pte
+runtime/executor/method.cpp:1487:9: runtime error: member call on null pointer of type 'flatbuffers::String'
+SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior runtime/executor/method.cpp:1487:9 in
+==<pid>== ERROR: libFuzzer: deadly signal
+    #0 ... (abort machinery)
+    Method::execute_instruction (method.cpp:1487)
+    Method::execute (method.cpp:1758)
+```
+
+**Reproduced 3/3 identical runs** (re-verified live for this report):
+```
+run 1: member call on null pointer of type 'flatbuffers::String'
+run 2: (identical)
+run 3: (identical)
+```
+
+Root cause was independently corroborated via a standalone minimal C++ program that loads the same `.pte`, confirms `op->overload()` is null, and reproduces the identical crash signature outside of the fuzzing harness entirely.
+
+## Impact
+
+**Who is affected:** Any application executing a method whose kernel fails at runtime for an operator with no `overload` field — this is triggered by ordinary, expected kernel error handling (shape mismatches, unsupported dtypes, etc.), not an artificial condition — **provided the deployment was built with `EXECUTORCH_ENABLE_LOGGING=ON`**.
+
+**What the attacker can do:** Cause a deterministic crash on operator failure, in builds with logging enabled.
+
+**What's at risk:** Availability only, and only in the logging-enabled configuration.
+
+**Why the conditional framing:** `EXECUTORCH_ENABLE_LOGGING` defaults to OFF in the CMake build (`ET_LOG_ENABLED=0`), which compiles the vulnerable call entirely out of the binary. However, this is not a safety net by design — it's an artifact of the logging macro's no-op expansion — and the flag is explicitly turned ON in numerous official ExecuTorch example/tutorial build scripts for XNNPACK, Vulkan, Qualcomm, Cadence, and Arm backends, plus the Llama example. Deployments following those documented instructions are affected.
+
+## Suggested Remediation
+
+```cpp
+auto op_index = instruction->instr_args_as_KernelCall()->op_index();
+ET_UNUSED auto op = serialization_plan_->operators()->Get(op_index);
+const char* overload_str =
+    (op->overload() != nullptr) ? op->overload()->c_str() : "";
+ET_LOG(
+    Error,
+    "KernelCall failed at instruction %" ET_PRIsize_t ":%" ET_PRIsize_t
+    " in operator %s.%s: 0x%x",
+    step_state_.chain_idx,
+    step_state_.instr_idx,
+    op->name()->c_str(),
+    overload_str,
+    (unsigned int)err);
+```
+
+This mirrors the existing, correct pattern already used by `populate_operator_name()` for the same field.
+
+**Design recommendation:** consider a small shared helper for extracting an operator's overload string safely, used by both `populate_operator_name()` and `execute_instruction()`'s failure-logging path, so this field's optionality doesn't need to be independently re-derived (and potentially missed) in future call sites.
+
+A regression test should register a deliberately-failing kernel for an `Operator` entry with no `overload` field, build with `EXECUTORCH_ENABLE_LOGGING=ON`, and assert `Method::execute()` returns a clean `Error` rather than crashing during failure logging.
+
+## Files Included in This Report
+
+- `poc/poc_operator_overload_null.pte` — the 368-byte PoC file (sha256 `f6b855b9d4bb69df0519e9954627703d88b9ef4da7104bbd52a6ba33239c3919`)
+- `poc/harness_execute_fuzzer.cpp` — the harness used to trigger and reproduce the crash, including fake kernel/backend registration for exercising `Method::execute()`
+
+## huntr Submission Note
+
+Per the huntr MFV program's submission requirements, this PoC needs to be uploaded to a public HuggingFace repository before filing. `poc/poc_operator_overload_null.pte` is ready for that upload. Given the conditional build-flag dependency, the submission should state this caveat explicitly per the analysis above.

poc/harness_execute_fuzzer.cpp

@@ -0,0 +1,174 @@
+// libFuzzer harness for Program::load_method() + Method::execute().
+// Registers a fake kernel (for every op name that might appear) that ALWAYS
+// calls context.fail(), to deterministically exercise the KernelCall failure
+// logging path in Method::execute_instruction() - specifically testing
+// whether op->overload()->c_str() is dereferenced unconditionally when
+// overload is null (optional per schema).
+//
+// Also registers a fake backend (matching common backend id strings) whose
+// init()/execute() are minimal, to exercise BackendDelegate::Init() and
+// GetProcessedData() paths without requiring a real backend implementation.
+#include <cstddef>
+#include <cstdint>
+#include <vector>
+
+#include <executorch/extension/data_loader/buffer_data_loader.h>
+#include <executorch/runtime/backend/interface.h>
+#include <executorch/runtime/executor/method.h>
+#include <executorch/runtime/executor/program.h>
+#include <executorch/runtime/core/memory_allocator.h>
+#include <executorch/runtime/core/hierarchical_allocator.h>
+#include <executorch/runtime/executor/memory_manager.h>
+#include <executorch/runtime/kernel/operator_registry.h>
+#include <executorch/runtime/kernel/kernel_runtime_context.h>
+#include <executorch/runtime/platform/runtime.h>
+
+using executorch::extension::BufferDataLoader;
+using executorch::runtime::Program;
+using executorch::runtime::Method;
+using executorch::runtime::MemoryAllocator;
+using executorch::runtime::HierarchicalAllocator;
+using executorch::runtime::MemoryManager;
+using executorch::runtime::Span;
+using executorch::runtime::EValue;
+using executorch::runtime::Error;
+using executorch::runtime::Result;
+using executorch::runtime::KernelRuntimeContext;
+using executorch::runtime::Kernel;
+using executorch::runtime::register_kernels;
+using executorch::runtime::BackendInterface;
+using executorch::runtime::Backend;
+using executorch::runtime::register_backend;
+using executorch::runtime::DelegateHandle;
+using executorch::runtime::FreeableBuffer;
+using executorch::runtime::ArrayRef;
+using executorch::runtime::CompileSpec;
+using executorch::runtime::BackendInitContext;
+using executorch::runtime::BackendExecutionContext;
+
+// A kernel that always fails - forces Method::execute_instruction()'s
+// KernelCall failure-logging path (which dereferences op->overload()) on
+// EVERY KernelCall instruction, regardless of which operator is named.
+void always_fail_kernel(KernelRuntimeContext& context, Span<EValue*> /*args*/) {
+  context.fail(Error::Internal);
+}
+
+// A minimal backend that succeeds trivially, to exercise
+// BackendDelegate::Init()/GetProcessedData() without needing a real backend.
+class FuzzTestBackend final : public BackendInterface {
+ public:
+  bool is_available() const override {
+    return true;
+  }
+  Result<DelegateHandle*> init(
+      BackendInitContext& /*context*/,
+      FreeableBuffer* /*processed*/,
+      ArrayRef<CompileSpec> /*compile_specs*/) const override {
+    return static_cast<DelegateHandle*>(nullptr);
+  }
+  Error execute(
+      BackendExecutionContext& /*context*/,
+      DelegateHandle* /*handle*/,
+      Span<EValue*> /*args*/) const override {
+    return Error::Ok;
+  }
+};
+
+static bool g_initialized = false;
+static FuzzTestBackend* g_backend = nullptr;
+
+extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size) {
+  if (!g_initialized) {
+    executorch::runtime::runtime_init();
+
+    // Register a wildcard-ish set of fallback kernels for common op names
+    // seen in the seed corpus, all pointing to always_fail_kernel. We can't
+    // truly wildcard-match in this registry, so register kernels for the
+    // most common ops that appear in the existing .pte seed corpus.
+    static const Kernel kernels[] = {
+        Kernel("aten::add.out", always_fail_kernel),
+        Kernel("aten::add.Tensor_out", always_fail_kernel),
+        Kernel("aten::mul.out", always_fail_kernel),
+        Kernel("aten::mul.Tensor_out", always_fail_kernel),
+        Kernel("aten::sub.out", always_fail_kernel),
+        Kernel("aten::relu.out", always_fail_kernel),
+        Kernel("aten::linear.out", always_fail_kernel),
+        Kernel("aten::cat.out", always_fail_kernel),
+        Kernel("aten::view_copy.out", always_fail_kernel),
+        Kernel("aten::index.Tensor_out", always_fail_kernel),
+        Kernel("aten::_softmax.out", always_fail_kernel),
+        Kernel("aten::convolution.out", always_fail_kernel),
+        Kernel("aten::permute_copy.out", always_fail_kernel),
+        Kernel("aten::_to_copy.out", always_fail_kernel),
+        // No-overload variants: matches Operator entries whose `overload`
+        // field is null/absent, which is the specific condition needed to
+        // test whether Method::execute_instruction()'s KernelCall failure
+        // path dereferences op->overload() unconditionally.
+        Kernel("aten::add", always_fail_kernel),
+        Kernel("aten::mul", always_fail_kernel),
+        Kernel("aten::sub", always_fail_kernel),
+        Kernel("aten::relu", always_fail_kernel),
+        Kernel("aten::linear", always_fail_kernel),
+        Kernel("aten::cat", always_fail_kernel),
+        Kernel("test_op", always_fail_kernel),
+        Kernel("test_op_no_overload", always_fail_kernel),
+    };
+    register_kernels({kernels, sizeof(kernels) / sizeof(kernels[0])});
+
+    static FuzzTestBackend backend_instance;
+    g_backend = &backend_instance;
+    static const char* backend_names[] = {
+        "XnnpackBackend", "CoreMLBackend", "QnnBackend",
+        "VulkanBackend", "MPSBackend", "TCE0", "TCE1"};
+    for (const char* name : backend_names) {
+      register_backend(Backend{name, g_backend});
+    }
+
+    g_initialized = true;
+  }
+
+  constexpr std::size_t kMaxInput = 8U * 1024U * 1024U;
+  if (data == nullptr || size == 0 || size > kMaxInput) {
+    return 0;
+  }
+
+  BufferDataLoader loader(data, size);
+  auto program = Program::load(&loader, Program::Verification::InternalConsistency);
+  if (!program.ok()) {
+    return 0;
+  }
+
+  static std::vector<uint8_t> method_pool(512 * 1024);
+  static std::vector<uint8_t> planned_pool(256 * 1024);
+  MemoryAllocator method_allocator(method_pool.size(), method_pool.data());
+  static Span<uint8_t> planned_span(planned_pool.data(), planned_pool.size());
+  HierarchicalAllocator planned_memory({&planned_span, 1});
+  MemoryManager mm(&method_allocator, &planned_memory, nullptr);
+
+  auto& p = program.get();
+  auto n = p.num_methods();
+  for (size_t i = 0; i < n; ++i) {
+    auto name = p.get_method_name(i);
+    if (!name.ok()) continue;
+    auto method = p.load_method(name.get(), &mm);
+    if (!method.ok()) {
+#ifdef ET_FUZZ_DEBUG
+      fprintf(stderr, "[dbg] load_method failed: 0x%x\n",
+              static_cast<unsigned int>(method.error()));
+#endif
+      continue;
+    }
+    // Attempt to set trivial inputs where possible, then execute. We don't
+    // try hard to satisfy every possible input shape - we're hunting for
+    // crashes during initialization and the KernelCall failure path, not
+    // testing correct numerical execution.
+    auto& m = method.get();
+    auto exec_err = m.execute();
+#ifdef ET_FUZZ_DEBUG
+    fprintf(stderr, "[dbg] execute() returned: 0x%x\n",
+            static_cast<unsigned int>(exec_err));
+#endif
+    (void)exec_err;
+  }
+  return 0;
+}