Skip to content
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

Fix ext_proc fuzzer test issues. #27619

Merged
merged 4 commits into from
May 26, 2023

Conversation

yanjunxiang-google
Copy link
Contributor

@yanjunxiang-google yanjunxiang-google commented May 24, 2023

Fix an ext_proc fuzzer issue.

The crash issue is due to some of the decoder_callbacks mock function are missing. Adding them.

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

@yanjunxiang-google yanjunxiang-google marked this pull request as draft May 24, 2023 23:56
Signed-off-by: Yanjun Xiang <[email protected]>
@yanjunxiang-google
Copy link
Contributor Author

ext_proc fuzzer is hitting below crash.

This is due to some of the mock functions of decoder_callbacks is missing, which causing the state.addBufferedData() is no-op. This issue is a fuzzer only issue.

[2023-05-25 17:25:25.880][4130221][debug][ext_proc] [source/extensions/filters/http/ext_proc/processor_state.cc:30] Traffic direction INBOUND: 200 ms timer enabled
[2023-05-25 17:25:25.880][4130221][debug][ext_proc] [source/extensions/filters/http/ext_proc/ext_proc.cc:176] Sending headers message
[2023-05-25 17:25:25.880][4130221][debug][ext_proc] [source/extensions/filters/http/ext_proc/ext_proc.cc:604] Processing mode overridden by server for this request
[2023-05-25 17:25:25.880][4130221][debug][ext_proc] [source/extensions/filters/http/ext_proc/ext_proc.cc:609] Received request headers response
[2023-05-25 17:25:25.880][4130221][debug][ext_proc] [source/extensions/filters/http/ext_proc/processor_state.cc:84] applying headers response. body mode = BUFFERED
[2023-05-25 17:25:25.880][4130221][debug][ext_proc] [source/extensions/filters/http/ext_proc/processor_state.cc:52] Traffic direction INBOUND: timer disabled
[2023-05-25 17:25:25.880][4130221][trace][ext_proc] [source/extensions/filters/http/ext_proc/ext_proc.cc:196] decodeHeaders returning 1
[2023-05-25 17:25:25.880][4130221][debug][misc] [./test/extensions/filters/http/common/fuzz/http_filter_fuzzer.h:103] Finished with FilterHeadersStatus: 1
[2023-05-25 17:25:25.880][4130221][debug][misc] [./test/extensions/filters/http/common/fuzz/http_filter_fuzzer.h:185] Decoding data (end_stream=true):
[2023-05-25 17:25:25.880][4130221][trace][ext_proc] [source/extensions/filters/http/ext_proc/ext_proc.cc:418] decodeData(4): end_stream = true
[2023-05-25 17:25:25.880][4130221][debug][ext_proc] [source/extensions/filters/http/ext_proc/ext_proc.cc:269] Sending request body message
[2023-05-25 17:25:25.880][4130221][debug][ext_proc] [source/extensions/filters/http/ext_proc/ext_proc.cc:270] yanjun before state.hasBufferedData() false
[2023-05-25 17:25:25.880][4130221][debug][ext_proc] [source/extensions/filters/http/ext_proc/ext_proc.cc:274] yanjun after state.hasBufferedData() false
source/extensions/filters/http/ext_proc/ext_proc.cc:275:28: runtime error: reference binding to null pointer of type 'const Buffer::Instance'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior source/extensions/filters/http/ext_proc/ext_proc.cc:275:28 in
AddressSanitizer:DEADLYSIGNAL

==4130221==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000028b982c bp 0x7ffeeda95fd0 sp 0x7ffeeda95c20 T0)
==4130221==The signal is caused by a READ memory access.
==4130221==Hint: address points to the zero page.
error: failed to decompress '.debug_aranges', zlib is not available
error: failed to decompress '.debug_info', zlib is not available
error: failed to decompress '.debug_abbrev', zlib is not available
error: failed to decompress '.debug_line', zlib is not available
error: failed to decompress '.debug_str', zlib is not available
error: failed to decompress '.debug_line_str', zlib is not available
error: failed to decompress '.debug_loclists', zlib is not available
error: failed to decompress '.debug_rnglists', zlib is not available
#0 0x28b982c in Envoy::Extensions::HttpFilters::ExternalProcessing::Filter::sendBodyChunk(Envoy::Extensions::HttpFilters::ExternalProcessing::ProcessorState&, Envoy::Buffer::Instance const&, Envoy::Extensions::HttpFilters::ExternalProcessing::ProcessorState::CallbackState, bool) /proc/self/cwd/source/extensions/filters/http/ext_proc/ext_proc.cc:523:3
#1 0x28b8236 in Envoy::Extensions::HttpFilters::ExternalProcessing::Filter::onData(Envoy::Extensions::HttpFilters::ExternalProcessing::ProcessorState&, Envoy::Buffer::Instance&, bool) /proc/self/cwd/source/extensions/filters/http/ext_proc/ext_proc.cc:275:7
#2 0x28bb7b4 in Envoy::Extensions::HttpFilters::ExternalProcessing::Filter::decodeData(Envoy::Buffer::Instance&, bool) /proc/self/cwd/source/extensions/filters/http/ext_proc/ext_proc.cc:419:23
#3 0x2847438 in Envoy::Http::FilterDataStatus Envoy::Extensions::HttpFilters::HttpFilterFuzzer::sendDataEnvoy::Http::StreamDecoderFilter(Envoy::Http::StreamDecoderFilter*, Envoy::Buffer::Instance&, bool) /proc/self/cwd/./test/extensions/filters/http/common/fuzz/http_filter_fuzzer.h:186:43
#4 0x274a98d in void Envoy::Extensions::HttpFilters::HttpFilterFuzzer::runDataEnvoy::Http::StreamDecoderFilter(Envoy::Http::StreamDecoderFilter*, test::fuzz::HttpData const&) /proc/self/cwd/./test/extensions/filters/http/common/fuzz/http_filter_fuzzer.h:115:30
#5 0x2739c35 in LLVMFuzzerTestOneInput /proc/self/cwd/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz.cc:112:10
#6 0x4fc595a in Envoy::(anonymous namespace)::FuzzerCorpusTest_RunOneCorpusFile_Test::TestBody() /proc/self/cwd/test/fuzz/main.cc:50:3
#7 0x768c4c7 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::)(), char const) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2580:10
#8 0x76624bb in testing::Test::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2655:5
#9 0x7663af0 in testing::TestInfo::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2832:11
#10 0x766510a in testing::TestSuite::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2986:28
#11 0x767e30b in testing::internal::UnitTestImpl::RunAllTests() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5697:44
#12 0x768fd17 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::)(), char const) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2580:10
#13 0x767d8bd in testing::UnitTest::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5280:10
#14 0x4fc3c95 in main /proc/self/cwd/external/com_google_googletest/googletest/include/gtest/gtest.h:2485:46
#15 0x7fd9e6be7189 (/lib/x86_64-linux-gnu/libc.so.6+0x27189) (BuildId: e144007f35d794adf218479af5ddcb2a11a2c583)
#16 0x7fd9e6be7244 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27244) (BuildId: e144007f35d794adf218479af5ddcb2a11a2c583)
#17 0x267b22d in _start (/usr/local/google/home/yanjunxiang/.cache/bazel/_bazel_yanjunxiang/ccb4eec3b43fef867ae6e7ed77595f24/execroot/envoy/bazel-out/k8-dbg/bin/test/extensions/filters/http/ext_proc/unit_test_fuzz/ext_proc_unit_test_fuzz+0x267b22d)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /proc/self/cwd/source/extensions/filters/http/ext_proc/ext_proc.cc:523:3 in Envoy::Extensions::HttpFilters::ExternalProcessing::Filter::sendBodyChunk(Envoy::Extensions::HttpFilters::ExternalProcessing::ProcessorState&, Envoy::Buffer::Instance const&, Envoy::Extensions::HttpFilters::ExternalProcessing::ProcessorState::CallbackState, bool)
==4130221==ABORTING

Signed-off-by: Yanjun Xiang <[email protected]>
@yanjunxiang-google
Copy link
Contributor Author

/assign @yanavlasov @adisuissa

@yanjunxiang-google yanjunxiang-google marked this pull request as ready for review May 25, 2023 18:28
Copy link
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cleaning up these issues!
Left a couple of questions.

ON_CALL(decoder_callbacks_, connection())
.WillByDefault(Return(OptRef<const Network::Connection>{connection_}));
connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress(addr_);
connection_.stream_info_.downstream_connection_info_provider_->setLocalAddress(addr_);
ON_CALL(decoder_callbacks_, addDecodedTrailers()).WillByDefault(ReturnRef(request_trailers_));
ON_CALL(encoder_callbacks_, addEncodedTrailers()).WillByDefault(ReturnRef(response_trailers_));
ON_CALL(decoder_callbacks_, addDecodedData(_, _)).WillByDefault(Return());
ON_CALL(encoder_callbacks_, addEncodedData(_, _)).WillByDefault(Return());
ON_CALL(decoder_callbacks_, decodingBuffer()).WillByDefault(Return(buffer_ptr_ = &buffer_));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason the assignment is being done as part of the Return statement?

I suggest returning the address of buffer_ directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

ON_CALL(decoder_callbacks_, connection())
.WillByDefault(Return(OptRef<const Network::Connection>{connection_}));
connection_.stream_info_.downstream_connection_info_provider_->setRemoteAddress(addr_);
connection_.stream_info_.downstream_connection_info_provider_->setLocalAddress(addr_);
ON_CALL(decoder_callbacks_, addDecodedTrailers()).WillByDefault(ReturnRef(request_trailers_));
ON_CALL(encoder_callbacks_, addEncodedTrailers()).WillByDefault(ReturnRef(response_trailers_));
ON_CALL(decoder_callbacks_, addDecodedData(_, _)).WillByDefault(Return());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I fully understand this, but I thought that the default behavior of a (Nice)Mock is to return the default value. Do these override a different ON_CALL somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah,no need. removed.

Signed-off-by: Yanjun Xiang <[email protected]>
@yanavlasov yanavlasov enabled auto-merge (squash) May 26, 2023 18:35
@yanavlasov yanavlasov merged commit 0e82260 into envoyproxy:main May 26, 2023
reskin89 pushed a commit to reskin89/envoy that referenced this pull request Jul 11, 2023
* Fix ext_proc fuzzer test issues.

Signed-off-by: Yanjun Xiang <[email protected]>
Signed-off-by: Ryan Eskin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants