Buffer overflow - in virtual recv function.

[ruehlchr]

Hi all,

I'd like to point out a bug:

1) a dynamic 'buffer' initialized with maxLen
2) the ::recv() method reads bytes into buffer and return nBytes that can be equal to maxLen
3) buffer[nByte] = \0 !!!!!! if the received bytes == maxLen we overflow and write a null
outside the barrier.. that can be cause seg-faults

Faulty Code

Code: Select all

       virtual int recv( std::string &msg, size_t maxLen ) const
        {
        char buffer[maxLen];
        int nBytes = 0;
                if ( (nBytes = ::recv( mSd, buffer, sizeof(buffer), 0 )) < 0 )
        {
            Debug( 1, "Recv of %d bytes max to string on sd %d failed: %s", maxLen, mSd, strerror(errno) );
            return( nBytes );
        }
        buffer[nBytes] = '\0';
        msg = buffer;
        return( nBytes );
        }

My suggestion

Code: Select all

       virtual int recv( std::string &msg, size_t maxLen ) const
        {
        char buffer[maxLen + 1];
        int32_t nBytes = 0;
                if ( (nBytes = ::recv( mSd, buffer, (uint32_t) maxLen , 0 )) < 0 )
        {
            Debug( 1, "Recv of %d bytes max to string on sd %d failed: %s", (uint32_t) maxLen, mSd, strerror(errno) );
            return( nBytes );
        }
        buffer[nBytes] = '\0';
        msg = buffer;
        return( nBytes );
        }

same same for the method using the msg.capacity()
File: zm_comms.h line 339 onwards

Please comment.

Chris

[bdmcnxn]

Yeah, the buffer print in case of buffer full, the Debug() macro (I think that's what it is) message print could easily case a segfault, if it's last char isn't '\0').

What's the patch submission policy here ??

PS> am pretty new to ZM, and have no code contribution so far.