Boost C++ Libraries: Ticket #7745: text_iarchive crashes on invalid data

component changed; cc, owner set

ivagulin@… — Wed, 28 Nov 2012 09:58:16 GMT

cc ivagulin@… added
owner set to Robert Ramey
component None → serialization

attachment set

ivagulin@… — Wed, 28 Nov 2012 10:00:38 GMT

attachment → text-iarchive-crash.patch

attached patch fixes problem for me

type changed

anonymous — Wed, 05 Dec 2012 23:18:25 GMT

type Bugs → Patches

Robert Ramey — Wed, 12 Dec 2012 21:56:58 GMT

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

Robert Ramey

Wed, 12 Dec 2012 22:28:48 GMT

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

Wed, 12 Dec 2012 22:33:54 GMT

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
    ...
}

status changed; resolution set

Robert Ramey — Thu, 13 Dec 2012 19:14:18 GMT

status new → closed
resolution → invalid

Sun, 16 Dec 2012 00:35:12 GMT

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?