-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[fuzz] fix fuzz crashes in fmt format #10935
Conversation
Signed-off-by: Asra Ali <[email protected]>
source/common/protobuf/utility.cc
Outdated
@@ -250,8 +250,8 @@ MissingFieldException::MissingFieldException(const std::string& field_name, | |||
|
|||
ProtoValidationException::ProtoValidationException(const std::string& validation_error, | |||
const Protobuf::Message& message) | |||
: EnvoyException(fmt::format("Proto constraint validation failed ({}): {}", validation_error, | |||
message.DebugString())) { | |||
: EnvoyException(absl::StrCat("Proto constraint validation failed (", validation_error, |
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.
@htuch FYI just limiting the validation_error
and message.DebugString()
to 10 characters doesn't solve the problem -- there's some other reason the the buffer allocation increases besides a large string
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.
Weird. Is it possible to get a heap profile?
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.
Yep, nothing obvious. I am starting to think this is a by-product of fuzz runner environment setup. Running all server config tests with -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION only produced failures in server/config fuzz tests, no other tests.
FYI the build fix (fmtlib/fmt#1650) will solve this problem and I could close out this PR if we patch fmt with this commit, but I think there is something weird if this is only happening in fuzz mode.
Some binary filenames not available. Symbolization may be incomplete.
Try setting PPROF_BINARY_PATH to the search path for local binaries.
File: server_fuzz_test
Type: inuse_space
Showing nodes accounting for 856.05kB, 98.73% of 867.06kB total
Dropped 170 nodes (cum <= 4.33kB)
flat flat% sum% cum cum%
219.88kB 25.36% 25.36% 219.88kB 25.36% std::allocator_traits::allocate
138.02kB 15.92% 41.28% 142.02kB 16.38% google::protobuf::DescriptorPool::Tables::AllocateBytes
120kB 13.84% 55.12% 120kB 13.84% std::basic_filebuf::_M_allocate_internal_buffer
88.41kB 10.20% 65.31% 150.37kB 17.34% google::protobuf::DescriptorPool::Tables::AllocateString
71kB 8.19% 73.50% 71kB 8.19% _GLOBAL__sub_I_eh_alloc.cc
48.75kB 5.62% 79.13% 48.75kB 5.62% std::__cxx11::basic_string::_M_construct
47.03kB 5.42% 84.55% 47.03kB 5.42% std::__cxx11::basic_string::_M_mutate
31.62kB 3.65% 88.20% 31.62kB 3.65% google::protobuf::DescriptorPool::Tables::AllocateEmptyString[abi:cxx11]
22.77kB 2.63% 90.82% 22.77kB 2.63% google::protobuf::Arena::CreateMaybeMessage
21.94kB 2.53% 93.35% 23.94kB 2.76% google::protobuf::DescriptorPool::Tables::AllocateMessage
15.67kB 1.81% 95.16% 27.45kB 3.17% testing::internal::MakeAndRegisterTestInfo
8.76kB 1.01% 96.17% 11.26kB 1.30% google::protobuf::DescriptorPool::Tables::AllocateFileTables
7.97kB 0.92% 97.09% 7.97kB 0.92% google::protobuf::internal::ExtensionSet::GrowCapacity
4.49kB 0.52% 97.61% 4.49kB 0.52% std::__cxx11::basic_string::reserve
4.38kB 0.5% 98.11% 4.38kB 0.5% google::protobuf::AssignDescriptorsHelper::AssignMessageDescriptor
2.30kB 0.27% 98.38% 7.05kB 0.81% testing::internal::TestMetaFactory::CreateTestFactory
1.84kB 0.21% 98.59% 10.73kB 1.24% testing::TestInfo::TestInfo
1.09kB 0.13% 98.72% 5.09kB 0.59% google::protobuf::DescriptorPool::Tables::AllocateOnceDynamic
0.08kB 0.009% 98.72% 401.13kB 46.26% Envoy::Fuzz::Runner::setupEnvironment
0.02kB 0.0027% 98.73% 11.27kB 1.30% Envoy::Logger::Registry::allLoggers
0.02kB 0.0027% 98.73% 10.63kB 1.23% std::__shared_count::__shared_count
0 0% 98.73% 12.74kB 1.47% Envoy::Logger::Context::Context
0 0% 98.73% 12.70kB 1.47% Envoy::Logger::Context::activate
0 0% 98.73% 11.27kB 1.30% Envoy::Logger::Registry::setLogLevel
0 0% 98.73% 10.55kB 1.22% Envoy::Logger::StandardLogger::StandardLogger
0 0% 98.73% 387.71kB 44.72% Envoy::ProcessWide::ProcessWide
0 0% 98.73% 387.59kB 44.70% Envoy::Server::validateProtoDescriptors
0 0% 98.73% 5.52kB 0.64% __gnu_cxx::new_allocator::construct
0 0% 98.73% 443.17kB 51.11% __libc_start_main
0 0% 98.73% 176.42kB 20.35% __pthread_once_slow
0 0% 98.73% 115.70kB 13.34% google::protobuf::(anonymous namespace)::AssignDescriptorsImpl
0 0% 98.73% 112.55kB 12.98% google::protobuf::DescriptorBuilder::AddSymbol
0 0% 98.73% 135.71kB 15.65% google::protobuf::DescriptorBuilder::AllocateArray
0 0% 98.73% 28.50kB 3.29% google::protobuf::DescriptorBuilder::AllocateNameString
0 0% 98.73% 57.92kB 6.68% google::protobuf::DescriptorBuilder::AllocateOptions
0 0% 98.73% 57.92kB 6.68% google::protobuf::DescriptorBuilder::AllocateOptionsImpl
0 0% 98.73% 38.29kB 4.42% google::protobuf::DescriptorBuilder::BuildEnum
0 0% 98.73% 30.29kB 3.49% google::protobuf::DescriptorBuilder::BuildEnumValue
0 0% 98.73% 247.65kB 28.56% google::protobuf::DescriptorBuilder::BuildField
0 0% 98.73% 247.65kB 28.56% google::protobuf::DescriptorBuilder::BuildFieldOrExtension
0 0% 98.73% 558.81kB 64.45% google::protobuf::DescriptorBuilder::BuildFile
0 0% 98.73% 558.78kB 64.45% google::protobuf::DescriptorBuilder::BuildFileImpl
0 0% 98.73% 439.13kB 50.65% google::protobuf::DescriptorBuilder::BuildMessage
0 0% 98.73% 6.11kB 0.7% google::protobuf::DescriptorBuilder::BuildMethod
0 0% 98.73% 6kB 0.69% google::protobuf::DescriptorBuilder::BuildOneof
0 0% 98.73% 11.01kB 1.27% google::protobuf::DescriptorBuilder::BuildService
0 0% 98.73% 52.70kB 6.08% google::protobuf::DescriptorBuilder::CrossLinkField
0 0% 98.73% 55.80kB 6.44% google::protobuf::DescriptorBuilder::CrossLinkFile
0 0% 98.73% 53.36kB 6.15% google::protobuf::DescriptorBuilder::CrossLinkMessage
0 0% 98.73% 558.81kB 64.45% google::protobuf::DescriptorPool::BuildFileFromDatabase
0 0% 98.73% 60.84kB 7.02% google::protobuf::DescriptorPool::CrossLinkOnDemandHelper
0 0% 98.73% 111.14kB 12.82% google::protobuf::DescriptorPool::FindFileByName
0 0% 98.73% 333.81kB 38.50% google::protobuf::DescriptorPool::FindMessageTypeByName
0 0% 98.73% 53.03kB 6.12% google::protobuf::DescriptorPool::FindMethodByName
0 0% 98.73% 55.94kB 6.45% google::protobuf::DescriptorPool::Tables::AddSymbol
0 0% 98.73% 139.05kB 16.04% google::protobuf::DescriptorPool::Tables::AllocateArray
0 0% 98.73% 447.68kB 51.63% google::protobuf::DescriptorPool::Tables::FindByNameHelper
0 0% 98.73% 111.14kB 12.82% google::protobuf::DescriptorPool::TryFindFileInFallbackDatabase
0 0% 98.73% 447.68kB 51.63% google::protobuf::DescriptorPool::TryFindSymbolInFallbackDatabase
0 0% 98.73% 60.84kB 7.02% google::protobuf::FieldDescriptor::InternalTypeOnceInit
0 0% 98.73% 60.84kB 7.02% google::protobuf::FieldDescriptor::TypeOnceInit
0 0% 98.73% 25.02kB 2.89% google::protobuf::FieldOptions::_InternalParse
0 0% 98.73% 64.68kB 7.46% google::protobuf::FileDescriptorTables::AddAliasUnderParent
0 0% 98.73% 4.80kB 0.55% google::protobuf::FileDescriptorTables::AddEnumValueByNumber
0 0% 98.73% 34.18kB 3.94% google::protobuf::FileDescriptorTables::AddFieldByNumber
0 0% 98.73% 156.80kB 18.08% google::protobuf::InsertIfNotPresent
0 0% 98.73% 22.77kB 2.63% google::protobuf::MessageLite::CreateMaybeMessage
0 0% 98.73% 33.98kB 3.92% google::protobuf::MessageLite::ParseFrom
0 0% 98.73% 33.98kB 3.92% google::protobuf::MessageLite::ParseFromString
0 0% 98.73% 7.97kB 0.92% google::protobuf::internal::ExtensionSet::Insert
0 0% 98.73% 7.97kB 0.92% google::protobuf::internal::ExtensionSet::MaybeNewExtension
0 0% 98.73% 14.66kB 1.69% google::protobuf::internal::ExtensionSet::MutableMessage
0 0% 98.73% 32.70kB 3.77% google::protobuf::internal::ExtensionSet::ParseField
0 0% 98.73% 32.70kB 3.77% google::protobuf::internal::ExtensionSet::ParseFieldWithExtensionInfo
0 0% 98.73% 33.98kB 3.92% google::protobuf::internal::MergePartialFromImpl
0 0% 98.73% 16.73kB 1.93% google::protobuf::internal::ParseContext::ParseMessage
0 0% 98.73% 443.20kB 51.12% main
0 0% 98.73% 35.98kB 4.15% std::_Hashtable::_M_allocate_buckets
0 0% 98.73% 156.80kB 18.08% std::_Hashtable::_M_insert
0 0% 98.73% 33.66kB 3.88% std::_Hashtable::_M_insert_unique_node
0 0% 98.73% 33.66kB 3.88% std::_Hashtable::_M_rehash
0 0% 98.73% 33.66kB 3.88% std::_Hashtable::_M_rehash_aux
0 0% 98.73% 50.85kB 5.86% std::_Vector_base::_M_allocate
0 0% 98.73% 9.91kB 1.14% std::__allocate_guarded
0 0% 98.73% 123.15kB 14.20% std::__detail::_AllocNode::operator()
0 0% 98.73% 35.98kB 4.15% std::__detail::_Hashtable_alloc::_M_allocate_buckets
0 0% 98.73% 123.15kB 14.20% std::__detail::_Hashtable_alloc::_M_allocate_node
0 0% 98.73% 156.80kB 18.08% std::__detail::_Insert_base::insert
0 0% 98.73% 176.54kB 20.36% std::__invoke
0 0% 98.73% 176.54kB 20.36% std::__invoke_impl
0 0% 98.73% 10.63kB 1.23% std::__shared_ptr::__shared_ptr
0 0% 98.73% 10.61kB 1.22% std::allocate_shared
0 0% 98.73% 5.45kB 0.63% std::allocator_traits::construct
0 0% 98.73% 115.70kB 13.34% std::call_once(std::once_flag&, void (&)(google::protobuf::internal::DescriptorTable const*), google::protobuf::internal::DescriptorTable const*&)::{lambda()#1}::operator()
0 0% 98.73% 115.70kB 13.34% std::call_once(std::once_flag&, void (&)(google::protobuf::internal::DescriptorTable const*), google::protobuf::internal::DescriptorTable const*&)::{lambda()#2}::__invoke
0 0% 98.73% 115.70kB 13.34% std::call_once(std::once_flag&, void (&)(google::protobuf::internal::DescriptorTable const*), google::protobuf::internal::DescriptorTable const*&)::{lambda()#2}::operator()
0 0% 98.73% 60.84kB 7.02% std::call_once(std::once_flag&, void (*&&)(google::protobuf::FieldDescriptor const*), google::protobuf::FieldDescriptor const*&&)::{lambda()#1}::operator()
0 0% 98.73% 60.76kB 7.01% std::call_once(std::once_flag&, void (*&&)(google::protobuf::FieldDescriptor const*), google::protobuf::FieldDescriptor const*&&)::{lambda()#2}::__invoke
0 0% 98.73% 60.76kB 7.01% std::call_once(std::once_flag&, void (*&&)(google::protobuf::FieldDescriptor const*), google::protobuf::FieldDescriptor const*&&)::{lambda()#2}::operator()
0 0% 98.73% 10.61kB 1.22% std::make_shared
0 0% 98.73% 10.63kB 1.23% std::shared_ptr::shared_ptr
0 0% 98.73% 156.80kB 18.08% std::unordered_map::insert
0 0% 98.73% 50.01kB 5.77% std::vector::_M_realloc_insert
0 0% 98.73% 53.66kB 6.19% std::vector::emplace_back
0 0% 98.73% 5.19kB 0.6% std::vector::push_back
0 0% 98.73% 34.75kB 4.01% testing::InitGoogleTest
0 0% 98.73% 34.75kB 4.01% testing::internal::InitGoogleTestImpl
0 0% 98.73% 4.74kB 0.55% testing::internal::ParameterizedTestFactory::ParameterizedTestFactory
0 0% 98.73% 34.50kB 3.98% testing::internal::ParameterizedTestSuiteInfo::RegisterTests
0 0% 98.73% 34.50kB 3.98% testing::internal::ParameterizedTestSuiteRegistry::RegisterTests
0 0% 98.73% 34.75kB 4.01% testing::internal::UnitTestImpl::PostFlagParsingInit
0 0% 98.73% 34.50kB 3.98% testing::internal::UnitTestImpl::RegisterParameterizedTests
0 0% 98.73% 5.34kB 0.62% validate::FieldRules::New
0 0% 98.73% 14.77kB 1.70% validate::FieldRules::_InternalParse
0 0% 98.73% 9.34kB 1.08% validate::FieldRules::_internal_mutable_string
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Friendly ping @asraa? I see fmtlib/fmt#1650 is merged, will that unblock this PR? |
I was having trouble profiling the dependency when I was running this test, because I'm wary that this might be something to avoid triggering generally. I will add an fmt.patch and revert the absl replacement |
Signed-off-by: Asra Ali <[email protected]>
Signed-off-by: Asra Ali <[email protected]>
Signed-off-by: Asra Ali <[email protected]>
Updated the PR description to reflect patch added. let me know what you think, thank you! |
@asraa since the patch is merged upstream, why don't we just bump our |
Signed-off-by: Asra Ali <[email protected]>
Signed-off-by: Asra Ali <[email protected]>
Signed-off-by: Asra Ali <[email protected]>
Done -- can't bump fmt to latest commit or sometime later, because otherwise spdlog doesn't build (is there some name clash? sdlog should pull in it's own fmt v6.1.2) |
urls = ["https://github.com/fmtlib/fmt/archive/6.0.0.tar.gz"], | ||
sha256 = "5014aacf55285bf79654539791de0d6925063fddf4dfdd597ef76b53eb994f86", | ||
strip_prefix = "fmt-e2ff910675c7800e5c4e28e1509ca6a50bdceafa", | ||
urls = ["https://github.com/fmtlib/fmt/archive/e2ff910675c7800e5c4e28e1509ca6a50bdceafa.tar.gz"], |
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.
Looks good, can you put the date above this line (as with other master pins in this file)?
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.
all set (i had some trouble with the quiche/boringssl pins, but can try to find their dates better)
Signed-off-by: Asra Ali <[email protected]>
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.
LGTM, thanks!
Commit description: Fixes fuzz crashes in fmt::format (https://github.com/fmtlib/fmt/blob/0463665ef136d685fe07a564d93c782456456d3d/include/fmt/format.h#L703) on certain invalid protobuf inputs. fmt is patches with PR (fmtlib/fmt#1650) that replaces the in-house fuzzing resource management to an fmt specific fuzzing macro.
Additional Description: The regression test added shows that the proto in question is not unreasonably huge for Envoy. This is causing a high unexplained crash percentage for many fuzz tests on OSS-Fuzz. Also bump fmt.
Testing: Added regression test in server fuzz test, failed
bazel test test/server:server_fuzz_test --copt=-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
beforeRelated: #9623
Risk level: Low
Signed-off-by: Asra Ali [email protected]