marzer / tomlplusplus

Header-only TOML config file parser and serializer for C++17.
https://marzer.github.io/tomlplusplus/
MIT License
1.58k stars 152 forks source link

fix stack overflow by limiting the maximum depth of dotted keys #242

Closed tyler92 closed 1 day ago

tyler92 commented 1 day ago

What does this change do?

This patch fixes potential stack overflow error in case of huge depths of dotted keys, like

[a.b.c.d. <very long list> .n]

This issue happens during fuzzing testing on google/oss-fuzz platform and prevents the fuzzer from achieving good coverage. Before the change:

$ examples/simple_parser crash-064bef61a4386fdb3e690f05a827389fe0c63b40 
Segmentation fault (core dumped)

with address sanitizer:

$ simple_parser crash-064bef61a4386fdb3e690f05a827389fe0c63b40 
AddressSanitizer:DEADLYSIGNAL
=================================================================
==327716==ERROR: AddressSanitizer: stack-overflow on address 0x7ffdcb974ff8 (pc 0x56059bbac291 bp 0x7ffdcb975090 sp 0x7ffdcb975000 T0)
    #0 0x56059bbac291 in std::_Rb_tree<toml::v3::key, std::pair<toml::v3::key const, std::unique_ptr<toml::v3::node, std::default_delete<toml::v3::node> > >, std::_Select1st<std::pair<toml::v3::key const, std::unique_ptr<toml::v3::node, std::default_delete<toml::v3::node> > > >, std::less<void>, std::allocator<std::pair<toml::v3::key const, std::unique_ptr<toml::v3::node, std::default_delete<toml::v3::node> > > > >::begin() /usr/include/c++/13/bits/stl_tree.h:997
    #1 0x56059bb997bf in std::map<toml::v3::key, std::unique_ptr<toml::v3::node, std::default_delete<toml::v3::node> >, std::less<void>, std::allocator<std::pair<toml::v3::key const, std::unique_ptr<toml::v3::node, std::default_delete<toml::v3::node> > > > >::begin() /usr/include/c++/13/bits/stl_map.h:369
    #2 0x56059bb8c31f in toml::v3::table::begin() /home/misha/work/relax/github.com/marzer/tomlplusplus/include/toml++/impl/table.hpp:799
    #3 0x56059bb8c31f in toml::v3::impl::impl_ex::parser::update_region_ends(toml::v3::node&) /home/misha/work/relax/github.com/marzer/tomlplusplus/include/toml++/impl/parser.inl:3509
    #4 0x56059bb8c3b1 in toml::v3::impl::impl_ex::parser::update_region_ends(toml::v3::node&) /home/misha/work/relax/github.com/marzer/tomlplusplus/include/toml++/impl/parser.inl:3512
    #5 0x56059bb8c3b1 in toml::v3::impl::impl_ex::parser::update_region_ends(toml::v3::node&) /home/misha/work/relax/github.com/marzer/tomlplusplus/include/toml++/impl/parser.inl:3512
    #6 0x56059bb8c3b1 in toml::v3::impl::impl_ex::parser::update_region_ends(toml::v3::node&) /home/misha/work/relax/github.com/marzer/tomlplusplus/include/toml++/impl/parser.inl:3512
    #7 0x56059bb8c3b1 in toml::v3::impl::impl_ex::parser::update_region_ends(toml::v3::node&) /home/misha/work/relax/github.com/marzer/tomlplusplus/include/toml++/impl/parser.inl:3512
...

After the change:

$ examples/simple_parser crash-064bef61a4386fdb3e690f05a827389fe0c63b40 
Error while parsing key: exceeded maximum dotted keys depth of 1024 (TOML_MAX_DOTTED_KEYS_DEPTH)
    (error occurred at line 1, column 2051 of 'crash-064bef61a4386fdb3e690f05a827389fe0c63b40')

Additionally the -j option has been added to zip command in fuzzing/build.sh -- without this option the seed corpus is useless because the fuzzer takes files only from the root level.

Is it related to an exisiting bug report or feature request?

No

Pre-merge checklist

marzer commented 1 day ago

Great work, thanks for contributing! 😄