Re: [reSIProcate] Problem using resipfaststream
I am still concerned about any situations that would cause resip::Data to
handle an absurdly large buffer. If the consensus is for only an assert,
then I think the assert should check for buffer sizes larger than what a
resip::Data should be handling (32k, 64k, 128k???). I will most likely
implement not only the assert but also the buffer truncation for my own app.
Thanks,
-justin
-----Original Message-----
From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx
[mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of david
Butcher
Sent: Friday, November 28, 2008 2:01 PM
To: Dario Bozzali
Cc: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: Re: [reSIProcate] Problem using resipfaststream
I would prefer to add an assertion to the Data constructors. Passing a
negative value as a length suggests a confused caller.
david
On Fri, Nov 28, 2008 at 1:17 AM, Dario Bozzali
<Dario.Bozzali@xxxxxxxxxxxxxxxxx> wrote:
> Hi all,
> I noticed that in Resiprocate main SVN head it was fixed the issue in
TlsConnection.cxx related to number of bytes returned by function SSL_read:
> int bytesRead = SSL_read(mSsl,buf,count);
> StackLog(<< "SSL_read returned " << bytesRead << " bytes [" <<
Data(Data::Borrow, buf, (bytesRead > 0)?(bytesRead):(0)) << "]");
>
> I also noticed that the same work-around was not applied to the following
lines (always in TlsConnection.cxx).
> restBytes = SSL_read(mSsl, buffer, SSL_pending(mSsl));
> StackLog(<< "SSL_read returned " << restBytes << " bytes [" <<
Data(Data::Borrow, buffer, restBytes) << "]");
> Is it not possible in this case that SSL_read() function returns -1 value?
>
> Any news regarding the fix proposed by Justin?
>
> I had a look at the code and in my opinion could be sufficient to change
the following Data constructors:
> - Data::Data(const char* str, int length)
> - Data::Data(const unsigned char* str, int length)
> - Data::Data(const char* str, int length, bool)
> - Data::Data(ShareEnum se, const char* buffer, int length)
> Changing mSize initialization from
> : mSize(length),
> to
> : mSize(length > 0 ? length : 0),
>
> What's your opinion regarding this change?
>
> Thank you in advance.
>
> Best regards,
> Dario.
> ________________________________
>
>
> -----Original Message-----
> From: Justin Matthews [mailto:jmatthewsr@xxxxxxxxx]
> Sent: martedì 4 novembre 2008 18.17
> To: Dario Bozzali; resiprocate-devel@xxxxxxxxxxxxxxx
> Subject: RE: [reSIProcate] Problem using resipfaststream
>
> Anyone have any objections/comments on making the changes described below
to
> resip::Data() constructors?
>
> Thanks,
>
> -justin
>
> -----Original Message-----
> From: Justin Matthews [mailto:jmatthewsr@xxxxxxxxx]
> Sent: Monday, November 03, 2008 8:06 AM
> To: 'Dario Bozzali'; 'resiprocate-devel@xxxxxxxxxxxxxxx'
> Subject: RE: [reSIProcate] Problem using resipfaststream
>
> Looks like the StackLog entry should be checking the return value from
SSL_read.
>
> Also, any resip::Data constructors that take an int (or any functions that
set Data::mSize) should be checking to see if the int is < 0 before
assigning mSize (unsigned).
>
> You can comment out the StackLog entry for now. Send any other errors
that you encounter.
>
> Thanks,
>
> -justin
>
> -----Original Message-----
> From: resiprocate-devel-bounces@xxxxxxxxxxxxxxx
> [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxx] On Behalf Of Dario
Bozzali
> Sent: Monday, November 03, 2008 6:48 AM
> To: resiprocate-devel@xxxxxxxxxxxxxxx
> Subject: [reSIProcate] Problem using resipfaststream
>
> Hi all,
> I encountered an issue using Resiprocate 1.4 (SVN head) using
resipfaststreams (define RESIP_USE_STL_STREAMS is commented).
> I obtained an access violation (using TLS, but I think that this is not
> important) I suppose because the read buffer size was -1.
> In TlsConnection.cxx there are the following two lines:
> int bytesRead = SSL_read(mSsl,buf,count);
> StackLog(<< "SSL_read returned " << bytesRead << " bytes [" <<
Data(Data::Borrow, buf, bytesRead) << "]"); Executing my application
SSL_read() returns -1.
> I suppose that the access violation is caused by operator << applied to
Data(Data::Borrow, buf, bytesRead), where bytesRead is -1.
> I had a look at "size_t DataBuffer::readbuf(char *buf, size_t count)"
> and "size_t DataBuffer::writebuf(const char *str, size_t count)" in
Resiprocate\rutil\DataStream.cxx that are used when RESIP_USE_STL_STREAMS is
not defined.
> The type of size_t is unsigned int but in readbuf() and writebuf() is
executed the following check:
> [...]
> if (count <= 0)
> {
> return 0;
> }
> [...]
> Actually if buffer size is -1, then count is 4294967295, so the method
doesn't exit and the writebuf() method causes access violation.
> Is that right or did I make a mistake?
>
> Thank you in advance.
>
> Best regards,
> Dario Bozzali.
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel@xxxxxxxxxxxxxxx
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel@xxxxxxxxxxxxxxx
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
_______________________________________________
resiprocate-devel mailing list
resiprocate-devel@xxxxxxxxxxxxxxx
https://list.resiprocate.org/mailman/listinfo/resiprocate-devel