Add more security checks to fix crashing during torch::jit::load#84343
Closed
apach301 wants to merge 1 commit intopytorch:masterfrom
Closed
Add more security checks to fix crashing during torch::jit::load#84343apach301 wants to merge 1 commit intopytorch:masterfrom
apach301 wants to merge 1 commit intopytorch:masterfrom
Conversation
Contributor
🔗 Helpful links
❌ 5 New FailuresAs of commit f68c4c1292 (more details on the Dr. CI page): Expand to see more
🕵️ 5 new failures recognized by patternsThe following CI failures do not appear to be due to upstream breakages
|
f68c4c1 to
86ea4ce
Compare
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/84343
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 1916e84: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Contributor
|
/easycla As part of the transition to the PyTorch Foundation, this project now requires contributions be covered under the new CLA. See #85559 for additional details. This comment will trigger a new check of this PR. If you are already covered, you will simply see a new "EasyCLA" check that passes. If you are not covered, a bot will leave a new comment with a link to sign. |
|
|
Contributor
|
@jerryzh168 Please take a look if this is something we want to do. |
Contributor
I'm not very familiar with this, cc @gmagogsfm can you take a look? |
Contributor
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
86ea4ce to
3a115fa
Compare
3a115fa to
1916e84
Compare
Contributor
Author
|
@gmagogsfm @kit1980 @eellison |
pytorchmergebot
pushed a commit
that referenced
this pull request
May 2, 2023
…94300) Hi! I've been fuzzing different pytorch modules, and found a crash inside one of them. Specifically, I'm talking about a module for unpickling and a function called `Unpickler::readInstruction()`. Running this function with provided crash file results in a crash, which occurs while calling `auto dict = stack_.at(dict_pos).toGenericDict();` [unpickler.cpp:561](https://github.com/pytorch/pytorch/blob/0e94fbc0c8ab1572c88159c1a4c397b6eb824c01/torch/csrc/jit/serialization/unpickler.cpp#L561). The crash occurs, because the index `dict_pos` is out of bounds (which itself happens because the stack size is 0). Besides this pull-request, there is another one related to unpickler hardening: #84343 All tests were performed on this pytorch version: [abc54f9](https://github.com/pytorch/pytorch/tree/abc54f93145830b502400faa92bec86e05422fbd) ### How to reproduce 1. To reproduce the crash, use provided docker: [Dockerfile](https://github.com/ispras/oss-sydr-fuzz/tree/master/projects/pytorch) 2. Build the container: `docker build -t oss-sydr-fuzz-pytorch-reproduce .` 3. Copy crash file to the current directory: - [crash-042dff5e121580425d9d34d0f293918f3c9fbf1e.zip](https://github.com/pytorch/pytorch/files/10674361/crash-042dff5e121580425d9d34d0f293918f3c9fbf1e.zip) 4. Run the container: ``docker run --privileged --network host -v `pwd`:/homedir --rm -it oss-sydr-fuzz-pytorch-reproduce /bin/bash`` 5. And execute the binary: `/message_deserialize_sydr /homedir/crash-042dff5e121580425d9d34d0f293918f3c9fbf1e` After execution completes you will see this error message: ```txt terminate called after throwing an instance of 'std::out_of_range' what(): vector::_M_range_check: __n (which is 18446744073709551613) >= this->size() (which is 0) ``` And this stacktrace: ```asan erminate called after throwing an instance of 'std::out_of_range' what(): vector::_M_range_check: __n (which is 18446744073709551613) >= this->size() (which is 0) ==39== ERROR: libFuzzer: deadly signal #0 0x5d0df1 in __sanitizer_print_stack_trace /llvm-project/compiler-rt/lib/asan/asan_stack.cpp:87:3 #1 0x545727 in fuzzer::PrintStackTrace() /llvm-project/compiler-rt/lib/fuzzer/FuzzerUtil.cpp:210:5 #2 0x52b933 in fuzzer::Fuzzer::CrashCallback() /llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:233:3 #3 0x7f9118e0341f (/lib/x86_64-linux-gnu/libpthread.so.0+0x1441f) #4 0x7f9118c2300a in raise (/lib/x86_64-linux-gnu/libc.so.6+0x4300a) #5 0x7f9118c02858 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x22858) #6 0x7f9119040910 (/lib/x86_64-linux-gnu/libstdc++.so.6+0x9e910) #7 0x7f911904c38b (/lib/x86_64-linux-gnu/libstdc++.so.6+0xaa38b) #8 0x7f911904c3f6 in std::terminate() (/lib/x86_64-linux-gnu/libstdc++.so.6+0xaa3f6) #9 0x7f911904c6a8 in __cxa_throw (/lib/x86_64-linux-gnu/libstdc++.so.6+0xaa6a8) #10 0x7f91190433aa (/lib/x86_64-linux-gnu/libstdc++.so.6+0xa13aa) #11 0x63acdf in std::vector<c10::IValue, std::allocator<c10::IValue> >::_M_range_check(unsigned long) const /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_vector.h:1073:4 #12 0xce8f93e in std::vector<c10::IValue, std::allocator<c10::IValue> >::at(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_vector.h:1094:2 #13 0xce8f93e in torch::jit::Unpickler::readInstruction() /pytorch_fuzz/torch/csrc/jit/serialization/unpickler.cpp:546:26 #14 0xce8d527 in torch::jit::Unpickler::run() /pytorch_fuzz/torch/csrc/jit/serialization/unpickler.cpp:235:27 #15 0xce8d1c2 in torch::jit::Unpickler::parse_ivalue() /pytorch_fuzz/torch/csrc/jit/serialization/unpickler.cpp:192:3 #16 0xcdf0792 in torch::jit::unpickle(std::function<unsigned long (char*, unsigned long)>, std::function<c10::StrongTypePtr (c10::QualifiedName const&)>, c10::ArrayRef<at::Tensor>, c10::Type::SingletonOrSharedTypePtr<c10::Type> (*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)) /pytorch_fuzz/torch/csrc/jit/serialization/pickle.cpp:127:20 #17 0xcdf104d in torch::jit::unpickle(char const*, unsigned long, std::function<c10::StrongTypePtr (c10::QualifiedName const&)>, c10::ArrayRef<at::Tensor>, c10::Type::SingletonOrSharedTypePtr<c10::Type> (*)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)) /pytorch_fuzz/torch/csrc/jit/serialization/pickle.cpp:137:10 #18 0xe0532db in torch::distributed::rpc::ScriptRemoteCall::fromMessage(torch::distributed::rpc::Message const&) /pytorch_fuzz/torch/csrc/distributed/rpc/script_remote_call.cpp:74:16 #19 0xe0ffa10 in torch::distributed::rpc::deserializeRequest(torch::distributed::rpc::Message const&) /pytorch_fuzz/torch/csrc/distributed/rpc/utils.cpp:108:14 #20 0x602a41 in LLVMFuzzerTestOneInput /message_deserialize_fuzz.cc:192:27 #21 0x52ce61 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15 #22 0x516d7c in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6 #23 0x51cacb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9 #24 0x546062 in main /llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 #25 0x7f9118c04082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) #26 0x51169d in _start (/message_deserialize_fuzz+0x51169d) NOTE: libFuzzer has rudimentary signal handlers. Combine libFuzzer with AddressSanitizer or similar for better crash reports. SUMMARY: libFuzzer: deadly signal ``` Pull Request resolved: #94300 Approved by: https://github.com/malfet, https://github.com/apach301
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These changes make loading models with torch::jit::load more stable. Proposed checks fixes multiple segmentation faults and heap buffer overflows that was found during fuzzing pytorch with sydr-fuzz.
Some of the fixed bugs:
crash-842314913bf1820ec19cddfbb7400ffdbb756920.zip
crash-e690c58718e88921350562f0b4d9180938145d77.zip
crash-ccd524e7ba19a37982dd91e0d6fc06bb26dd0b10.zip
Some other crashes found by fuzzer:
crash-0cab888cbd1e9fea92ab6ddeadf40b958b87d62b.zip
crash-04c9ba8e3b0f15028fd0fb0ed014fd352e182a1d.zip
crash-422ad8c3a3472980ba751f4c7f79cf2b53e49927.zip
cc @EikanWang @jgong5 @wenzhe-nrv @sanchitintel