Bug 21 - TlsConnection buffer allocation broken for >16k messages
Summary: TlsConnection buffer allocation broken for >16k messages
Status: NEW
Alias: None
Product: resiprocate
Classification: Unclassified
Component: stack (libresip) (show other bugs)
Version: unspecified
Hardware: Other Linux
: P1 critical
Assignee: Owner of all unassigned bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-17 06:49 CDT by mironoz
Modified: 2010-06-21 04:19 CDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description mironoz 2010-06-17 06:49:03 CDT
Noticed on  1.4-1.6 releases


While processing messages with large content that exceeds the 16k boundary

TlsConnection::read() reallocates current buffer by calling getWriteBufferForExtraBytes() even though the current buffer is not filled in by the last SSL_read()

This results in non initialized bytes from the end of the current buffer being placed in to the message content.
Comment 1 mironoz 2010-06-21 04:19:08 CDT
Here is the patch that will keep current buffer around until it is filled in

--- resiprocate/resip/stack/ssl/TlsConnection.cxx      2009-12-14 09:56:23.000000000 +0100
+++ resiprocate-patch/resip/stack/ssl/TlsConnection.cxx 2010-06-17 14:13:27.000000000 +0200
@@ -352,11 +352,12 @@
    }
 
    int bytesRead = SSL_read(mSsl,buf,count);
-   StackLog(<< "SSL_read returned " << bytesRead << " bytes [" << Data(Data::Borrow, buf, (bytesRead > 0)?(bytesRead):(0)) << "]");
+   StackLog(<< "SSL_read returned " << bytesRead << " bytes [" << Data(Data::Borrow, buf, (bytesRead > 0)?(bytesRead):(0)) << "] count =" << count);
 
    int bytesPending = SSL_pending(mSsl);
 
-   if ((bytesRead > 0) && (bytesPending > 0))
+   // ZZZ   if ((bytesRead > 0) && (bytesPending > 0))
+   if ((bytesRead > 0) && (bytesPending > 0) && isWriteBufferFull(bytesRead)) // ZZZ only realloc at full buffer
    {
       char* buffer = getWriteBufferForExtraBytes(bytesPending);
       if (buffer)
--- resiprocate/resip/stack/ConnectionBase.hxx 2008-07-25 22:54:42.000000000 +0200
+++ resiprocate-patch/resip/stack/ConnectionBase.hxx    2010-06-17 14:13:05.000000000 +0200
@@ -68,6 +68,8 @@
       void decompressNewBytes(int bytesRead, Fifo<TransactionMessage>& fifo);
       std::pair<char*, size_t> getWriteBuffer();
       char* getWriteBufferForExtraBytes(int extraBytes);
+  //  ZZZ
+  bool isWriteBufferFull( size_t extraBytes) { return mBufferPos + extraBytes >= mBufferSize; }
       
       // for avoiding copies in external transports--not used in core resip
       void setBuffer(char* bytes, int count);