Qt4-preview-feedback Archive, May 2007
Buffer overrun in QSslSocket::readData()
Message 1 in thread
I believe I've found a buffer overrun in QSslSocket::readData() which was
causing random crashes for me. Its definition is as follows (taken from
snapshot 20070508):
qint64 QSslSocket::readData(char *data, qint64 maxlen)
{
Q_D(QSslSocket);
qint64 readBytes = 0;
if (d->mode == UnencryptedMode && !d->autoStartHandshake) {
readBytes = d->plainSocket->read(data, maxlen);
} else {
do {
const char *readPtr = d->readBuffer.readPointer();
int bytesToRead = qMin<int>(maxlen,
d->readBuffer.nextDataBlockSize());
::memcpy(data + readBytes, readPtr, bytesToRead);
readBytes += bytesToRead;
d->readBuffer.free(bytesToRead);
} while (!d->readBuffer.isEmpty());
}
#ifdef QSSLSOCKET_DEBUG
qDebug() << "QSslSocket::readData(" << (void *)data << "," << maxlen <<
") ==" << readBytes;
#endif
return readBytes;
}
The problem is that although the qMin will prevent it from reading more than
maxlen bytes and hence overrunning the output buffer pointed to by data, it
doesn't take into account how much data has already been read, so the memcpy
can overrun the buffer if the loop makes more than one iteration and maxlen
bytes have already been read.
The solution I've used is:
int bytesToRead = qMin<int>(maxlen - readBytes,
d->readBuffer.nextDataBlockSize());
I also find that the loop won't terminate if there's still data left to read
but the output buffer is full, so I also changed:
} while (!d->readBuffer.isEmpty() && maxlen > readBytes);
--
[ signature omitted ]
Message 2 in thread
Mark Sawle wrote:
> I believe I've found a buffer overrun in QSslSocket::readData() which was
> causing random crashes for me. Its definition is as follows (taken from
> snapshot 20070508):
Thanks for this report; both bugs have just been fixed.
--
[ signature omitted ]