Re: [Boost-users] [review][fixed_string] FixedString review starts today

From: Peter Dimov (pdimov_at_[hidden])
Date: 2019-12-02 19:20:27

Zach Laine wrote:

> Ok, so the issue as you see it is that a program that is memory unsafe 
> should die in release builds rather than allowing an attacker to take 
> over, is that right?

Yes, this is known as converting a remote execution exploit into a denial of 
service exploit. Not great, not terrible.

> If so, I understand the motivation, but I don't share it -- I literally do 
> not ever need to worry about such things in my work.  Should everyone pay 
> the checking cost, because this is a concern for some code (even if it is 
> a common concern -- I'm not minimizing it more than to say that it's <100% 
> of uses).

It's a very, very common concern; it was actually the #1 security issue (and 
maybe still is.) As I said, this was the primary motivation behind 
Microsoft's security initiative with their _s functions, and they took this 
exact same approach - all strcpy-like functions check for buffer overrun and 
terminate. This is a practically unquestioned security best practice.

> Moreover, you mention using __builtin_trap below, which gives you a quick 
> death instead of acting as an attack vector, but the proposed design does 
> not.  Again I ask, if we throw in this case, what do I write in catching 
> code to remediate the situation that I need to stuff 10lbs of bits into a 
> 5lb sack?

It's hard to answer this without context. The simplest possible case would 
be

void append_something( char const * p, std::size_t n, error_code & ec ) 
noexcept
{
    if( buffer_.size() + n > buffer_.max_size() )
    {
        ec = make_error_code( errc::invalid_argument );
        return;
    }

    buffer_.append( p, n );
    ec = {};
}

This goes straight to terminate regardless of whether append aborts or 
throws, because of the noexcept. And yes, this can happen, because I have a 
bug in my size check, as 99% of all people would.

In a system that uses exceptions instead of error codes, the functions won't 
be noexcept, but then you'd just let the length_error bubble up, as with any 
other failure. If you for instance try to serialize a 10lb json into a 
fixed_string<5lb>, the serialization will fail with an exception and you'll 
need to figure out what to do with that failure, as with any other. This is 
no different from giving a fixed storage allocator to some std::vector (or 
to the JSON deserializer, if you will.)