-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not allow to read tensor's external_data outside the model directory #4400
Conversation
Signed-off-by: jnovikov <johnnovikov0@gmail.com>
Signed-off-by: jnovikov <johnnovikov0@gmail.com>
Signed-off-by: jnovikov <johnnovikov0@gmail.com>
Signed-off-by: jnovikov <johnnovikov0@gmail.com>
Signed-off-by: jnovikov <johnnovikov0@gmail.com>
Signed-off-by: jnovikov <johnnovikov0@gmail.com>
Signed-off-by: jnovikov <johnnovikov0@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit comments. Thank you for the contribution to make ONNX more secure!
Signed-off-by: jnovikov <johnnovikov0@gmail.com>
Signed-off-by: jnovikov <johnnovikov0@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for making ONNX more secure! Please remove pathlib
if it is really unused. Otherwise LGTM.
Signed-off-by: jnovikov <johnnovikov0@gmail.com>
…ry (onnx#4400) * Not allow to read tensor external_data outside the model directory Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix formatting errors Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Disable segfaulty test Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix cpp tests Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix UB while removing ../ Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix clang-format Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Check for symlinks only on POSIX systems Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Add specific to Windows external_data test Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Change specific Windows external_data test decorator tofix mypy Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Remove unused pathlib Signed-off-by: jnovikov <johnnovikov0@gmail.com> Signed-off-by: jnovikov <johnnovikov0@gmail.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com>
) * Do not allow to read tensor's external_data outside the model directory (#4400) * Not allow to read tensor external_data outside the model directory Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix formatting errors Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Disable segfaulty test Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix cpp tests Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix UB while removing ../ Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix clang-format Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Check for symlinks only on POSIX systems Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Add specific to Windows external_data test Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Change specific Windows external_data test decorator tofix mypy Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Remove unused pathlib Signed-off-by: jnovikov <johnnovikov0@gmail.com> Signed-off-by: jnovikov <johnnovikov0@gmail.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * cherry pick #4470 Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * resolve Windows CI issue: use fixed Python 3.10.5 Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> * fix flake8 Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> Signed-off-by: jnovikov <johnnovikov0@gmail.com> Signed-off-by: Chun-Wei Chen <jacky82226@gmail.com> Co-authored-by: Ivan Novikov <johnnovikov0@gmail.com>
…ry (onnx#4400) * Not allow to read tensor external_data outside the model directory Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix formatting errors Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Disable segfaulty test Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix cpp tests Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix UB while removing ../ Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Fix clang-format Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Check for symlinks only on POSIX systems Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Add specific to Windows external_data test Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Change specific Windows external_data test decorator tofix mypy Signed-off-by: jnovikov <johnnovikov0@gmail.com> * Remove unused pathlib Signed-off-by: jnovikov <johnnovikov0@gmail.com> Signed-off-by: jnovikov <johnnovikov0@gmail.com>
Signed-off-by: jnovikov johnnovikov0@gmail.com
This PR fixes the vulnerability that allows to read tensor_data outside the model directory.
The logic behind the PR is to first normalize the relative_path inside the tensor and then check that path has no '../' suffix.
Normalization code is based on go-lang path.Clean() but simplified to work correctly with the relative_paths only.
Provided tests should cover most of the cases.
Please note that 'optimal' solution would be to use boost::filesystem or c++17 filesystem to get the absolute path file and resolve possible symlink problems, but AFAIK the project target is C++11 and the boost is not used at the project.
fixes #3991