From 5b77af41be15a233c5da5d9ac97d909976ab647c Mon Sep 17 00:00:00 2001 From: Gopalakrishnan Nallasamy Date: Fri, 26 Jun 2026 17:10:01 -0700 Subject: [PATCH 1/3] EPContext sample helper: reject .. leaf names and add zero-copy read IsDirectoryOrEmptyName now also rejects a leaf .. (e.g. sub/..) so untrusted model-derived directory-like names are rejected up front instead of failing later as a confusing file I/O error. Non-leaf .. stays handled by ContainsPathTraversal and the model-directory containment check. Add a move-only EpContextData owning buffer plus ReadEpContextData overloads: the app read-callback path now adopts the allocator-provided buffer instead of copying it into a std::vector, and the file path reads straight into the owned vector. The std::vector ReadEpContextDataWithFileFallback overloads remain as convenience wrappers. The example EP uses the zero-copy path. Adds a zero-copy unit test and a sub/.. rejection test. --- .../test/autoep/ep_context_data_utils_test.cc | 39 +++- .../autoep/library/ep_context_data_utils.h | 180 ++++++++++++++---- .../autoep/library/example_plugin_ep/ep.cc | 8 +- 3 files changed, 191 insertions(+), 36 deletions(-) diff --git a/onnxruntime/test/autoep/ep_context_data_utils_test.cc b/onnxruntime/test/autoep/ep_context_data_utils_test.cc index 8425280e4845b..084f39324d0ec 100644 --- a/onnxruntime/test/autoep/ep_context_data_utils_test.cc +++ b/onnxruntime/test/autoep/ep_context_data_utils_test.cc @@ -162,11 +162,16 @@ TEST(OrtEpLibrary, EpContextDataUtils_ResolvePathRejectsUnsafeNames) { empty_model_path_graph.ToExternal(), data_path), ORT_INVALID_ARGUMENT, "requires a model path"); - // A model-derived name that designates a directory ("." or a trailing separator with an empty filename) is + // A model-derived name that designates a directory (".", "..", or a trailing separator with an empty filename) is // rejected up front, rather than resolving to a directory and failing later with a confusing I/O error. ExpectOrtStatusError(ep_context_data_utils::ResolveEpContextDataPath(api, ".", empty_model_path_graph.ToExternal(), data_path), ORT_INVALID_ARGUMENT, "must refer to a file"); + // A leaf ".." (e.g. "sub/..") resolves to the parent/model directory; it is rejected here rather than passing the + // model-directory containment check and surfacing later as a directory I/O failure. + ExpectOrtStatusError(ep_context_data_utils::ResolveEpContextDataPath(api, "sub/..", + empty_model_path_graph.ToExternal(), data_path), + ORT_INVALID_ARGUMENT, "must refer to a file"); ExpectOrtStatusError(ep_context_data_utils::WriteEpContextDataWithFileFallback( api, nullptr, ".", "unused.ctx", nullptr, nullptr, 0), ORT_INVALID_ARGUMENT, "must refer to a file"); @@ -323,5 +328,37 @@ TEST(OrtEpLibrary, EpContextDataUtils_ReadCallbackRejectsNullBufferForNonEmptyPa EXPECT_EQ(read_callback_state.read_file_name, "invalid_callback_context.bin"); } +TEST(OrtEpLibrary, EpContextDataUtils_ReadEpContextDataAdoptsCallbackBufferZeroCopy) { + const auto& api = Ort::GetApi(); + + // Callback path: ReadEpContextData adopts the buffer the callback allocates (no copy into a std::vector) and frees + // it via the same allocator when the EpContextData owner is destroyed. + EpContextDataCallbackState read_callback_state; + read_callback_state.payload = {'z', 'e', 'r', 'o', '-', 'c', 'o', 'p', 'y'}; + ep_context_data_utils::EpContextData owned; + ASSERT_ORTSTATUS_OK(ep_context_data_utils::ReadEpContextData( + api, LoadEpContextDataCallback, &read_callback_state, "zero_copy_context.bin", nullptr, owned)); + ASSERT_TRUE(read_callback_state.read_called); + EXPECT_EQ(read_callback_state.read_file_name, "zero_copy_context.bin"); + ASSERT_EQ(owned.size(), read_callback_state.payload.size()); + EXPECT_EQ(std::vector(owned.data(), owned.data() + owned.size()), read_callback_state.payload); + + // File-fallback path: with no callback configured, ReadEpContextData reads the file into the owned buffer. + const std::filesystem::path test_dir = PrepareTempTestDir("ort_ep_context_data_utils_zero_copy_test"); + auto cleanup = gsl::finally([&]() { std::filesystem::remove_all(test_dir); }); + + const std::string payload = "zero copy file payload"; + const std::filesystem::path data_path = test_dir / "zero_copy_file.bin"; + std::string data_file_name; + ASSERT_ORTSTATUS_OK(ep_context_data_utils::PathToUtf8String(api, data_path, data_file_name)); + ASSERT_ORTSTATUS_OK(ep_context_data_utils::WriteEpContextDataToFile(api, data_file_name.c_str(), nullptr, + payload.data(), payload.size())); + + ep_context_data_utils::EpContextData file_owned; + ASSERT_ORTSTATUS_OK(ep_context_data_utils::ReadEpContextData( + api, /*ep_context_config=*/nullptr, data_file_name.c_str(), nullptr, file_owned)); + EXPECT_EQ(std::string(file_owned.data(), file_owned.data() + file_owned.size()), payload); +} + } // namespace test } // namespace onnxruntime diff --git a/onnxruntime/test/autoep/library/ep_context_data_utils.h b/onnxruntime/test/autoep/library/ep_context_data_utils.h index a3f9a377ee92f..40fbb97101cd7 100644 --- a/onnxruntime/test/autoep/library/ep_context_data_utils.h +++ b/onnxruntime/test/autoep/library/ep_context_data_utils.h @@ -32,11 +32,13 @@ // the ORT C and EP ABI and are provided as a reference for EP authors that need to handle external (non-embedded) // EPContext binary data. // -// The intended entry points for EP implementers are the ReadEpContextDataWithFileFallback / -// WriteEpContextDataWithFileFallback overloads: they prefer an application-supplied OrtReadNamedBufferFunc / -// OrtWriteNamedBufferFunc (carried by OrtEpContextConfig) and fall back to file I/O when no callback is configured. -// The other functions are lower-level building blocks. Production EPs should additionally apply their own sandboxing, -// size limits, and path policies; see the per-function notes on how untrusted, model-derived names are treated. +// The intended entry points for EP implementers are the ReadEpContextData / WriteEpContextDataWithFileFallback +// overloads: they prefer an application-supplied OrtReadNamedBufferFunc / OrtWriteNamedBufferFunc (carried by +// OrtEpContextConfig) and fall back to file I/O when no callback is configured. ReadEpContextData returns an owning +// EpContextData buffer and avoids copying large data; ReadEpContextDataWithFileFallback is a std::vector +// convenience wrapper around it. The other functions are lower-level building blocks. Production EPs should +// additionally apply their own sandboxing, size limits, and path policies; see the per-function notes on how +// untrusted, model-derived names are treated. namespace ep_context_data_utils { #ifdef _WIN32 @@ -163,12 +165,13 @@ inline bool HasAbsoluteOrRootedPath(const std::filesystem::path& path) { } // Returns true if the final component of `path` is empty (e.g., a trailing separator like "sub/") or is the -// current-directory entry ".", i.e. the name designates a directory rather than a file (".." is handled separately by -// ContainsPathTraversal()). Such a name resolves to a directory and would only surface later as a confusing file I/O -// failure, so model-derived names like these are rejected up front. +// current-directory entry "." or the parent-directory entry "..", i.e. the name designates a directory rather than a +// file. Such a name resolves to a directory (e.g. "sub/.." resolves to the parent/model directory) and would only +// surface later as a confusing file I/O failure, so model-derived names like these are rejected up front. A non-leaf +// ".." anywhere in the path is rejected separately by ContainsPathTraversal() / the model-directory containment check. inline bool IsDirectoryOrEmptyName(const std::filesystem::path& path) { const std::filesystem::path leaf = path.filename(); - return leaf.empty() || leaf == std::filesystem::path{"."}; + return leaf.empty() || leaf == std::filesystem::path{"."} || leaf == std::filesystem::path{".."}; } // Returns true if `candidate_full` (a base-relative name already combined with `base`) resolves to a location inside @@ -348,53 +351,166 @@ inline OrtStatus* WriteEpContextDataToFile(const OrtApi& api, const char* file_n return WriteEpContextDataToResolvedFile(api, data_path, buffer, buffer_size); } -// Low-level overload that takes the read callback and its opaque state directly. Production EPs should use the -// overload below that takes an OrtEpContextConfig; this overload exists so unit tests can inject a callback without -// constructing an OrtEpContextConfig. When `read_func` is null the data is read from a file. -inline OrtStatus* ReadEpContextDataWithFileFallback( - const OrtApi& api, - OrtReadNamedBufferFunc read_func, void* read_state, - const char* file_name, const OrtGraph* graph, - std::vector& data) { +// Forward declaration so EpContextData can grant the populating read helper access to its internals. +class EpContextData; +inline OrtStatus* ReadEpContextData(const OrtApi& api, OrtReadNamedBufferFunc read_func, void* read_state, + const char* file_name, const OrtGraph* graph, EpContextData& out); + +// RAII owner for the bytes returned by an EPContext read, used to avoid copying potentially large data. The +// app-supplied read-callback path adopts the allocator-provided buffer directly (no copy) and frees it via the same +// allocator on destruction; the file-fallback path owns the bytes in a std::vector read straight from disk. Either +// way the bytes are accessed through data()/size() without an extra copy. EPs that handle large EPContext blobs +// should prefer ReadEpContextData() + EpContextData; the std::vector ReadEpContextDataWithFileFallback() +// overloads remain as a convenience for callers (e.g. tests) that want an owned vector and can afford one copy. +class EpContextData { + public: + EpContextData() = default; + ~EpContextData() { FreeAllocatorBuffer(); } + + EpContextData(EpContextData&& other) noexcept { MoveFrom(other); } + EpContextData& operator=(EpContextData&& other) noexcept { + if (this != &other) { + FreeAllocatorBuffer(); + MoveFrom(other); + } + return *this; + } + + EpContextData(const EpContextData&) = delete; + EpContextData& operator=(const EpContextData&) = delete; + + // Pointer to the bytes, owned by this object (valid until it is destroyed or reassigned). May be null only when + // size() == 0. + const char* data() const noexcept { + return buffer_ != nullptr ? static_cast(buffer_) : file_bytes_.data(); + } + size_t size() const noexcept { return buffer_ != nullptr ? buffer_size_ : file_bytes_.size(); } + bool empty() const noexcept { return size() == 0; } + + private: + friend OrtStatus* ReadEpContextData(const OrtApi& api, OrtReadNamedBufferFunc read_func, void* read_state, + const char* file_name, const OrtGraph* graph, EpContextData& out); + + void FreeAllocatorBuffer() noexcept { + if (buffer_ != nullptr && allocator_ != nullptr && api_ != nullptr) { + // Best-effort free; release any returned status without throwing (matches the OrtStatus*-based, exception-free + // style of these helpers). The default allocator is owned by ORT and must not be released here. + Ort::Status free_status{api_->AllocatorFree(allocator_, buffer_)}; + static_cast(free_status); + } + api_ = nullptr; + allocator_ = nullptr; + buffer_ = nullptr; + buffer_size_ = 0; + } + + void Reset() noexcept { + FreeAllocatorBuffer(); + file_bytes_.clear(); + } + + void MoveFrom(EpContextData& other) noexcept { + api_ = other.api_; + allocator_ = other.allocator_; + buffer_ = other.buffer_; + buffer_size_ = other.buffer_size_; + file_bytes_ = std::move(other.file_bytes_); + other.api_ = nullptr; + other.allocator_ = nullptr; + other.buffer_ = nullptr; + other.buffer_size_ = 0; + } + + const OrtApi* api_ = nullptr; // Used to free buffer_ (callback path); null otherwise. + OrtAllocator* allocator_ = nullptr; // Allocator that owns buffer_ (callback path); null for the file path. + void* buffer_ = nullptr; // Adopted callback buffer; null when the bytes live in file_bytes_. + size_t buffer_size_ = 0; + std::vector file_bytes_; // Owns the bytes on the file-fallback path. +}; + +// Zero-copy read: reads EPContext binary data named `file_name` into `out` (reset first). If `read_func` is non-null +// it is invoked and the buffer it allocates is adopted by `out` (no copy); otherwise the data is read from the file +// fallback into `out`. See ReadEpContextDataFromFile() for how `graph` governs name resolution. This low-level +// overload takes the callback directly so tests can inject one; production EPs use the OrtEpContextConfig overload. +inline OrtStatus* ReadEpContextData(const OrtApi& api, OrtReadNamedBufferFunc read_func, void* read_state, + const char* file_name, const OrtGraph* graph, EpContextData& out) { + out.Reset(); + if (file_name == nullptr || file_name[0] == '\0') { return api.CreateStatus(ORT_INVALID_ARGUMENT, "EPContext data file name must not be empty"); } if (read_func == nullptr) { - return ReadEpContextDataFromFile(api, file_name, graph, data); + // No callback: read the file fallback straight into the owned vector (no extra copy). + return ReadEpContextDataFromFile(api, file_name, graph, out.file_bytes_); } // Use the C allocator API (not Ort::AllocatorWithDefaultOptions, whose constructor throws) so this OrtStatus*-based // helper stays exception-free. The default allocator is owned by ORT and must not be released here. OrtAllocator* allocator = nullptr; RETURN_IF_ERROR(api.GetAllocatorWithDefaultOptions(&allocator)); + void* ep_context_data = nullptr; size_t ep_context_data_size = 0; OrtStatus* status = read_func(read_state, file_name, allocator, &ep_context_data, &ep_context_data_size); - auto buffer_deleter = [&api, allocator](void* buffer_to_free) { - if (buffer_to_free != nullptr) { - // Best-effort free during cleanup; release any returned status without throwing. - Ort::Status free_status{api.AllocatorFree(allocator, buffer_to_free)}; - static_cast(free_status); - } - }; - std::unique_ptr ep_context_data_guard(ep_context_data, buffer_deleter); + // Adopt whatever the callback allocated so `out` frees it via the allocator on destruction, even on the error paths + // below. Ownership transfers without copying the (potentially large) buffer. + out.api_ = &api; + out.allocator_ = allocator; + out.buffer_ = ep_context_data; + out.buffer_size_ = ep_context_data_size; if (status != nullptr) { return status; } if (ep_context_data_size != 0 && ep_context_data == nullptr) { - return api.CreateStatus( - ORT_FAIL, "OrtReadNamedBufferFunc returned a null buffer for non-empty EPContext data"); + return api.CreateStatus(ORT_FAIL, "OrtReadNamedBufferFunc returned a null buffer for non-empty EPContext data"); } - data.clear(); - if (ep_context_data != nullptr) { - const char* ep_context_data_begin = static_cast(ep_context_data); - data.assign(ep_context_data_begin, ep_context_data_begin + ep_context_data_size); + return nullptr; +} + +// Zero-copy read using the OrtReadNamedBufferFunc carried by `ep_context_config` (if any); otherwise reads the file +// fallback. See ReadEpContextData() above and ReadEpContextDataFromFile() for `graph` handling. +inline OrtStatus* ReadEpContextData(const OrtApi& api, const OrtEpContextConfig* ep_context_config, + const char* file_name, const OrtGraph* graph, EpContextData& out) { + OrtReadNamedBufferFunc read_func = nullptr; + void* read_state = nullptr; + if (ep_context_config != nullptr) { + auto get_read_func = + Ort::Experimental::Get_OrtEpApi_EpContextConfig_GetEpContextDataReadFunc_SinceV28_Fn(&api); + if (get_read_func == nullptr) { + return api.CreateStatus(ORT_NOT_IMPLEMENTED, + "OrtEpApi_EpContextConfig_GetEpContextDataReadFunc is not available"); + } + RETURN_IF_ERROR(get_read_func(ep_context_config, &read_func, &read_state)); + } + return ReadEpContextData(api, read_func, read_state, file_name, graph, out); +} + +// std::vector convenience overload that takes the read callback and its opaque state directly. Production EPs +// should use the OrtEpContextConfig overload below; this overload exists so unit tests can inject a callback without +// constructing an OrtEpContextConfig. When `read_func` is null the data is read from a file straight into `data` (no +// extra copy); the callback path reads zero-copy via ReadEpContextData() and then copies once into `data`. EPs +// handling large data that want to avoid that copy should call ReadEpContextData() and consume the EpContextData +// buffer directly. +inline OrtStatus* ReadEpContextDataWithFileFallback( + const OrtApi& api, + OrtReadNamedBufferFunc read_func, void* read_state, + const char* file_name, const OrtGraph* graph, + std::vector& data) { + if (file_name == nullptr || file_name[0] == '\0') { + return api.CreateStatus(ORT_INVALID_ARGUMENT, "EPContext data file name must not be empty"); + } + + if (read_func == nullptr) { + return ReadEpContextDataFromFile(api, file_name, graph, data); } + EpContextData owned; + RETURN_IF_ERROR(ReadEpContextData(api, read_func, read_state, file_name, graph, owned)); + data.assign(owned.data(), owned.data() + owned.size()); return nullptr; } diff --git a/onnxruntime/test/autoep/library/example_plugin_ep/ep.cc b/onnxruntime/test/autoep/library/example_plugin_ep/ep.cc index 90ad4e7976824..64973acf5d920 100644 --- a/onnxruntime/test/autoep/library/example_plugin_ep/ep.cc +++ b/onnxruntime/test/autoep/library/example_plugin_ep/ep.cc @@ -424,9 +424,11 @@ OrtStatus* ORT_API_CALL ExampleEp::CompileImpl(_In_ OrtEp* this_ptr, _In_ const // This example only exercises the load-side read flow (callback first, file fallback otherwise) to show how // an EP retrieves EPContext binary data during compile. A real EP would consume `ep_context_data` (e.g., - // initialize a kernel/engine from it); here it is intentionally read and then discarded. - std::vector ep_context_data; - RETURN_IF_ERROR(ep_context_data_utils::ReadEpContextDataWithFileFallback( + // initialize a kernel/engine from it via ep_context_data.data()/size()); here it is intentionally read and + // then discarded. ReadEpContextData returns an owning EpContextData buffer that adopts the callback-provided + // memory instead of copying it. + ep_context_data_utils::EpContextData ep_context_data; + RETURN_IF_ERROR(ep_context_data_utils::ReadEpContextData( ep->ort_api, ep->ep_context_config_.get(), ep_cache_context.c_str(), ort_graphs[0], ep_context_data)); } From 43696765fbd0f4ff6bf5c1844fe80d60b61bb60b Mon Sep 17 00:00:00 2001 From: Gopalakrishnan Nallasamy Date: Fri, 26 Jun 2026 17:43:21 -0700 Subject: [PATCH 2/3] EPContext sample helper: drop coarse traversal guard from trusted path resolution Addresses #28624 review feedback. Now that IsResolvedPathWithinBase() does real model-directory containment for untrusted model-relative names, the lexical ContainsPathTraversal() guard is no longer applied on the trusted graph==nullptr branch of ResolveEpContextDataPath(): trusted callers already may pass absolute paths and own their paths, so there is no model directory to contain against. ContainsPathTraversal() is kept solely for ValidateEpContextDataName(), which validates the logical callback-namespace name written into the model ep_cache_context attribute (never resolved against a filesystem base). Updates the two trusted-branch tests; logical-name and model-directory containment coverage is unchanged. --- .../test/autoep/ep_context_data_utils_test.cc | 11 +++++--- .../autoep/library/ep_context_data_utils.h | 25 +++++++++++-------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/onnxruntime/test/autoep/ep_context_data_utils_test.cc b/onnxruntime/test/autoep/ep_context_data_utils_test.cc index 084f39324d0ec..adec22f2f9aa0 100644 --- a/onnxruntime/test/autoep/ep_context_data_utils_test.cc +++ b/onnxruntime/test/autoep/ep_context_data_utils_test.cc @@ -127,9 +127,10 @@ TEST(OrtEpLibrary, EpContextDataUtils_ResolvePathRejectsUnsafeNames) { const auto& api = Ort::GetApi(); std::filesystem::path data_path; - ExpectOrtStatusError(ep_context_data_utils::ResolveEpContextDataPath(api, "../escape.ctx", nullptr, data_path), - ORT_INVALID_ARGUMENT, "EPContext data file name must not contain path traversal"); - EXPECT_TRUE(data_path.empty()); + // Trusted direct callers (graph == nullptr) own the path: ".." is allowed (there is no model directory to contain + // against, and absolute paths are already permitted) and the name is accepted as-is rather than rejected. + ASSERT_ORTSTATUS_OK(ep_context_data_utils::ResolveEpContextDataPath(api, "../escape.ctx", nullptr, data_path)); + EXPECT_FALSE(data_path.empty()); #ifdef _WIN32 const char* absolute_file_name = "C:\\temp\\escape.ctx"; @@ -151,8 +152,10 @@ TEST(OrtEpLibrary, EpContextDataUtils_ResolvePathRejectsUnsafeNames) { #endif std::vector data; + // Trusted (graph == nullptr) reads also allow ".."; the resolver no longer rejects it, so a non-existent target now + // surfaces as a normal file-open failure rather than a traversal rejection. ExpectOrtStatusError(ep_context_data_utils::ReadEpContextDataFromFile(api, "../escape.ctx", nullptr, data), - ORT_INVALID_ARGUMENT, "EPContext data file name must not contain path traversal"); + ORT_FAIL, "Failed to open EPContext data file for read"); ExpectOrtStatusError(ep_context_data_utils::WriteEpContextDataWithFileFallback( api, nullptr, absolute_file_name, "unused.ctx", nullptr, nullptr, 0), ORT_INVALID_ARGUMENT, "EPContext data file name must not be absolute or rooted"); diff --git a/onnxruntime/test/autoep/library/ep_context_data_utils.h b/onnxruntime/test/autoep/library/ep_context_data_utils.h index 40fbb97101cd7..91ec704f93a27 100644 --- a/onnxruntime/test/autoep/library/ep_context_data_utils.h +++ b/onnxruntime/test/autoep/library/ep_context_data_utils.h @@ -145,11 +145,13 @@ inline std::string PathToUtf8StringForMessage(const std::filesystem::path& path) return status.IsOK() ? utf8_path : std::string{""}; } -// Lexical check for a ".." component. This is a coarse guard used when there is no filesystem base directory to -// contain against: logical callback-namespace names and trusted (graph == nullptr) physical paths. It is NOT a -// containment mechanism: it does not resolve symlinks and it rejects benign cases such as "a/b/c/../file.txt". -// Filesystem containment against a model directory is done by IsResolvedPathWithinBase() below, which the untrusted -// (model-relative) resolution path uses. +// Lexical check for a ".." component. Used only by ValidateEpContextDataName() to reject ".." in the logical +// callback-namespace name that is written into the EPContext model's ep_cache_context attribute. That logical name is +// never resolved against a filesystem base, so the model-directory containment check (IsResolvedPathWithinBase() +// below) cannot apply to it; rejecting ".." up front keeps a generated model from embedding an unsafe relative +// reference. It is NOT a containment mechanism: it does not resolve symlinks and it rejects benign cases such as +// "a/b/c/../file.txt". Filesystem containment against a model directory is done by IsResolvedPathWithinBase(), which +// the untrusted (model-relative) resolution path uses. inline bool ContainsPathTraversal(const std::filesystem::path& path) { const std::filesystem::path parent_dir{".."}; for (const auto& component : path) { @@ -232,8 +234,9 @@ inline OrtStatus* ValidateEpContextDataName(const OrtApi& api, const char* file_ // Resolves `file_name` to a filesystem path for reading or writing EPContext data (used by both the read path and // the write-fallback path). // -// When `graph` is null the caller is trusted and owns the path: `file_name` is returned as-is and may be absolute (a -// lexical ".." is still rejected as a coarse guard). When `graph` is non-null, `file_name` originates from the +// When `graph` is null the caller is trusted and owns the path: `file_name` is returned as-is and may be absolute and +// may contain ".." (there is no model directory to contain against, and absolute paths are already permitted, so no +// traversal check is applied). When `graph` is non-null, `file_name` originates from the // untrusted EPContext model "ep_cache_context" attribute: the graph must have a model path, the name must be // relative, and after combining it with the model's directory the result must stay within that directory. Symlinks and // ".." are resolved (via weakly_canonical), so a name that escapes the model directory - including through a symlink - @@ -255,11 +258,11 @@ inline OrtStatus* ResolveEpContextDataPath(const OrtApi& api, const char* file_n return api.CreateStatus(ORT_INVALID_ARGUMENT, "EPContext data file name is not a valid path"); } - // Trusted direct callers (graph == nullptr) own the path and may pass an absolute physical path. + // Trusted direct callers (graph == nullptr) own the path and may pass an absolute physical path, including one with + // ".." components. There is no model directory to contain against here, and absolute paths are already allowed, so + // no traversal check is applied; the untrusted (model-relative) branch below is the one constrained to the model + // directory. if (graph == nullptr) { - if (ContainsPathTraversal(candidate_path)) { - return api.CreateStatus(ORT_INVALID_ARGUMENT, "EPContext data file name must not contain path traversal"); - } data_path = candidate_path; return nullptr; } From 60d1b620f8bf93b128b5141f7ea3fd872ec18b83 Mon Sep 17 00:00:00 2001 From: Gopalakrishnan Nallasamy Date: Fri, 26 Jun 2026 19:35:57 -0700 Subject: [PATCH 3/3] EPContext sample helper: address latest review comments Clear the vector-output convenience wrapper before invoking the callback reader and avoid pointer arithmetic for empty EpContextData buffers by assigning only when non-empty. Update traversal comments to describe model-relative non-leaf .. as canonicalized and containment-checked rather than always rejected. Add focused tests for empty callback payloads and stale-output clearing on callback failure. --- .../test/autoep/ep_context_data_utils_test.cc | 10 ++++++++++ .../test/autoep/library/ep_context_data_utils.h | 15 +++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/onnxruntime/test/autoep/ep_context_data_utils_test.cc b/onnxruntime/test/autoep/ep_context_data_utils_test.cc index adec22f2f9aa0..f0d5168c5f7c4 100644 --- a/onnxruntime/test/autoep/ep_context_data_utils_test.cc +++ b/onnxruntime/test/autoep/ep_context_data_utils_test.cc @@ -296,6 +296,14 @@ TEST(OrtEpLibrary, EpContextDataUtils_CallbackFallbackUsesCallbacks) { EXPECT_EQ(read_callback_state.read_file_name, "callback_context.bin"); EXPECT_EQ(data, read_callback_state.payload); + read_callback_state = {}; + data.assign({'s', 't', 'a', 'l', 'e'}); + ASSERT_ORTSTATUS_OK(ep_context_data_utils::ReadEpContextDataWithFileFallback( + api, LoadEpContextDataCallback, &read_callback_state, "empty_callback_context.bin", nullptr, data)); + ASSERT_TRUE(read_callback_state.read_called); + EXPECT_EQ(read_callback_state.read_file_name, "empty_callback_context.bin"); + EXPECT_TRUE(data.empty()); + const std::string payload = "callback write payload"; ASSERT_ORTSTATUS_OK(ep_context_data_utils::WriteEpContextDataWithFileFallback( api, StoreEpContextDataCallback, &write_callback_state, "callback_write_context.bin", @@ -323,12 +331,14 @@ TEST(OrtEpLibrary, EpContextDataUtils_ReadCallbackRejectsNullBufferForNonEmptyPa EpContextDataCallbackState read_callback_state; std::vector data; + data.assign({'s', 't', 'a', 'l', 'e'}); ExpectOrtStatusError(ep_context_data_utils::ReadEpContextDataWithFileFallback( api, LoadInvalidEpContextDataCallback, &read_callback_state, "invalid_callback_context.bin", nullptr, data), ORT_FAIL, "OrtReadNamedBufferFunc returned a null buffer for non-empty EPContext data"); ASSERT_TRUE(read_callback_state.read_called); EXPECT_EQ(read_callback_state.read_file_name, "invalid_callback_context.bin"); + EXPECT_TRUE(data.empty()); } TEST(OrtEpLibrary, EpContextDataUtils_ReadEpContextDataAdoptsCallbackBufferZeroCopy) { diff --git a/onnxruntime/test/autoep/library/ep_context_data_utils.h b/onnxruntime/test/autoep/library/ep_context_data_utils.h index 91ec704f93a27..3a9b0db96e4cc 100644 --- a/onnxruntime/test/autoep/library/ep_context_data_utils.h +++ b/onnxruntime/test/autoep/library/ep_context_data_utils.h @@ -170,7 +170,8 @@ inline bool HasAbsoluteOrRootedPath(const std::filesystem::path& path) { // current-directory entry "." or the parent-directory entry "..", i.e. the name designates a directory rather than a // file. Such a name resolves to a directory (e.g. "sub/.." resolves to the parent/model directory) and would only // surface later as a confusing file I/O failure, so model-derived names like these are rejected up front. A non-leaf -// ".." anywhere in the path is rejected separately by ContainsPathTraversal() / the model-directory containment check. +// ".." in a logical callback-namespace name is rejected by ContainsPathTraversal(); in model-relative filesystem +// names it is canonicalized and accepted only if the resolved path stays within the model directory. inline bool IsDirectoryOrEmptyName(const std::filesystem::path& path) { const std::filesystem::path leaf = path.filename(); return leaf.empty() || leaf == std::filesystem::path{"."} || leaf == std::filesystem::path{".."}; @@ -511,16 +512,22 @@ inline OrtStatus* ReadEpContextDataWithFileFallback( return ReadEpContextDataFromFile(api, file_name, graph, data); } + // Clear up front so `data` is empty on any error path below and for an empty callback payload; assign only when the + // adopted buffer is non-empty (avoids pointer arithmetic on a possibly-null empty buffer). + data.clear(); EpContextData owned; RETURN_IF_ERROR(ReadEpContextData(api, read_func, read_state, file_name, graph, owned)); - data.assign(owned.data(), owned.data() + owned.size()); + if (!owned.empty()) { + data.assign(owned.data(), owned.data() + owned.size()); + } return nullptr; } // Reads EPContext binary data named `file_name`. If the session configured an OrtReadNamedBufferFunc (carried by // `ep_context_config`), it is used; otherwise the data is read from a file. When `graph` is non-null it is the -// EPContext model graph: untrusted absolute/rooted/traversal names are rejected and relative names are resolved -// against the model directory. Pass `graph == nullptr` only for trusted callers supplying a physical path. `data` is +// EPContext model graph: untrusted absolute/rooted names are rejected and relative names are resolved against the +// model directory, with canonicalization/containment handling any ".." components. Pass `graph == nullptr` only for +// trusted callers supplying a physical path. `data` is // cleared first and receives the bytes on success. inline OrtStatus* ReadEpContextDataWithFileFallback( const OrtApi& api,