text_iarchive crashes on invalid data

[anonymous]

Following code randomly trigger OOM if "in" parameter not starting with number.

template <typename Type>
void
from_string(const std::string in, Type &out)
{
    std::stringstream ss(in);
    boost::archive::text_iarchive ia(ss);
    ia >> boost::serialization::make_nvp("obj", out);
}

This is linux box(gcc compiler and libstdc++) so real allocation starts in memset, not in new. Here is backtrace which cause it.

#0  0x0000003a48c7a203 in memset () from /lib64/libc.so.6
#1  0x0000003fa449cce2 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::append(unsigned long, char) () from /usr/lib64/libstdc++.so.6
#2  0x00002aaaaab03b89 in resize (this=0x7fffffffe3a0, s="")
    at /usr/lib/gcc/x86_64-redhat-linux/4.1.2/../../../../include/c++/4.1.2/bits/basic_string.h:629
#3  boost::archive::text_iarchive_impl<boost::archive::text_iarchive>::load (
    this=0x7fffffffe3a0, s="") at ./boost/archive/impl/text_iarchive_impl.ipp:55
#4  0x00002aaaaab03c44 in load_primitive<boost::archive::text_iarchive, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x2aab59734fb0)
    at ./boost/archive/detail/iserializer.hpp:107
#5  invoke<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > (
    this=0x2aab59734fb0) at ./boost/archive/detail/iserializer.hpp:338
#6  invoke<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > (
    this=0x2aab59734fb0) at ./boost/archive/detail/iserializer.hpp:415
#7  load<boost::archive::text_iarchive, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > (this=0x2aab59734fb0) at ./boost/archive/detail/iserializer.hpp:554
#8  load_override<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > (
    this=0x2aab59734fb0) at ./boost/archive/detail/common_iarchive.hpp:61
#9  load_override<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > (
    this=0x2aab59734fb0) at ./boost/archive/basic_text_iarchive.hpp:62
#10 load_override<std::basic_string<char, std::char_traits<char>, std::allocator<char> > > (
    this=0x2aab59734fb0) at ./boost/archive/text_iarchive.hpp:66
#11 operator>><std::basic_string<char, std::char_traits<char>, std::allocator<char> > > (
    this=0x2aab59734fb0) at ./boost/archive/detail/interface_iarchive.hpp:61
#12 boost::archive::basic_text_iarchive<boost::archive::text_iarchive>::init (
    this=0x2aab59734fb0) at ./boost/archive/impl/basic_text_iarchive.ipp:50
#13 0x00002aaaaab03fb8 in boost::archive::text_iarchive_impl<boost::archive::text_iarchive>::text_iarchive_impl (this=0x7fffffffe3a0, is=<value optimized out>, flags=0)
    at ./boost/archive/impl/text_iarchive_impl.ipp:123
#14 0x000000000043fe16 in boost::archive::text_iarchive::text_iarchive (this=0x7fffffffe3a0, 
    is_=..., flags=0) at /usr/include/boost/archive/text_iarchive.hpp:115
#15 0x0000000000440549 in from_string<log_info_t> 

[ivagulin@…]

attached patch fixes problem for me

[Robert Ramey]

There should be no need to initialize the size variable. Perhaps the archive file was corrupted in some way.

Robert Ramey

[Igor Vagulin <ivagulin@…>]

Yes, archive was incorrect(it was "nonexistent-key", redis-cplusplus-client use it as mark of non existent key). I expect some kind of exception, but I get program which tries to allocate 230 of ram. With my patch I get std::bad_alloc. I believe it is much better behaviour.

Maybe my explanations not very clear. Here what I get step by step:

text_iarchive_impl<Archive>::load(std::string &s){

std::size_t size; here size set to some junk from stack, in my bad case it was -1

• this->This() >> size; libstdc++, gcc4.4 didn't set size, it still ~230 is.get(); s.resize(size); allocation of size set to -1 ...

}

Only downside of this patch I can imagine is mostly unneeded initialization of variable, but:

• I wasn't able to measure it
• Increase in stability outweight this in my opinion

[Igor Vagulin <ivagulin@…>]

Sorry for bad format, here it is properly formated.

text_iarchive_impl<Archive>::load(std::string &s){
    std::size_t size; //here size set to some junk from stack, in my bad case it was -1
    this->This() >> size; //libstdc++, gcc4.4 didn't set size, it still ~2**30
    is.get(); 
    s.resize(size); //allocation of size set to -1
    ... 
} 

[Igor Vagulin <ivagulin@…>]

Can you please provide rationale for bug invalidation. Is there any better way to check for archive validity other than trying to extract object from it?