Add path traversal validation#2222
Conversation
There was a problem hiding this comment.
Pull request overview
This PR strengthens security in Model::CreateSessionOptionsFromConfig by validating the custom_ops_library config value before attempting to resolve and load a custom ops shared library, aiming to prevent loading arbitrary libraries via unsafe paths.
Changes:
- Added rejection of absolute
custom_ops_librarypaths. - Added rejection of path traversal components (
..) incustom_ops_library.
| std::string custom_library_file_prefix = config_session_options.custom_ops_library.value(); | ||
|
|
||
| // If relative path, try to resolve using multiple search locations | ||
| // Reject absolute paths and any rooted paths (e.g., Windows drive-relative "C:foo.dll"), |
There was a problem hiding this comment.
I have seen a mix of absolute and relative paths used for the path to the custom ops library. Is it possible to support both?
There was a problem hiding this comment.
Yes, we now support both absolute and relative paths. However, to prevent arbitrary library loading (e.g., a malicious C:\evil\malware.dll), we validate that the final resolved path falls within one of the trusted directories: the model folder, the EP library directory, or the current working directory. Absolute paths that point outside these locations are rejected. Path traversal (..) is also blocked upfront. This gives users flexibility while maintaining security.
This pull request introduces an important security enhancement to the way custom operation libraries are loaded in the
Model::CreateSessionOptionsFromConfigfunction. Specifically, it adds validation to ensure that only relative paths without path traversal are accepted, preventing potential security vulnerabilities from loading arbitrary libraries.Security improvements for custom library loading:
custom_ops_libraryconfiguration, throwing a runtime error if an absolute path is provided...) forcustom_ops_library, throwing a runtime error if detected.