diff options
author | Marc Mutz <marc.mutz@qt.io> | 2022-04-04 14:48:42 +0200 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2022-04-05 20:46:40 +0000 |
commit | ceb236954afce1885d236b34c7ade8acc7ccb011 (patch) | |
tree | 145223900ef48a8389e7d3bc963ee9491d71259a | |
parent | 921a27575081fa0e72151df2a4455e8e3c252fb6 (diff) |
QBuffer: fail early in seek() beyond QByteArray's max capacity
On 32-bit platforms, the range of qsizetype is smaller than the range
of the qint64 used as a parameter in seek().
When seek()ing beyond the current buffer's size, the old code relied
on a write() to fill the gap with NUL bytes. This has two problems:
First, this may allocate a huge amount of memory just to find that it
cannot write that much, possibly even taking the program down when the
allocation in the QByteArray ctor fails, instead of returning false from
seek().
Second, the QByteArray ctor to which we pass the gapSize only takes
qsizetype, not qint64, so we were writing data of size gapSize mod
(INT_MAX+1) on 32-bit platforms, which may succeed, just to find that
that wasn't the number of bytes we expected to be written. By that
time, however, the internal buffer has already been enlarged.
Fix by checking whether the desired seek position is within the limits
that QByteArray can contain early on, before attempting to construct
such a large QByteArray.
[ChangeLog][QtCore][QBuffer] Fixed silent data corruption on 32-bit
platforms when seek() fails due to position > INT_MAX.
Fixes: QTBUG-102274
Change-Id: Ib63cef7e7e61ef8101a5f056c7b2198bb7baa228
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 4bc85b9850303fa20206f8774af88d72593d3454)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
-rw-r--r-- | src/corelib/io/qbuffer.cpp | 6 | ||||
-rw-r--r-- | tests/auto/corelib/io/qbuffer/tst_qbuffer.cpp | 24 |
2 files changed, 28 insertions, 2 deletions
diff --git a/src/corelib/io/qbuffer.cpp b/src/corelib/io/qbuffer.cpp index 8bf51709bc..1b34eed876 100644 --- a/src/corelib/io/qbuffer.cpp +++ b/src/corelib/io/qbuffer.cpp @@ -366,7 +366,9 @@ qint64 QBuffer::size() const bool QBuffer::seek(qint64 pos) { Q_D(QBuffer); - if (pos > d->buf->size() && isWritable()) { + const auto oldBufSize = d->buf->size(); + constexpr qint64 MaxSeekPos = (std::numeric_limits<decltype(oldBufSize)>::max)(); + if (pos <= MaxSeekPos && pos > oldBufSize && isWritable()) { if (seek(d->buf->size())) { const qint64 gapSize = pos - d->buf->size(); if (write(QByteArray(gapSize, 0)) != gapSize) { @@ -377,7 +379,7 @@ bool QBuffer::seek(qint64 pos) return false; } } else if (pos > d->buf->size() || pos < 0) { - qWarning("QBuffer::seek: Invalid pos: %d", int(pos)); + qWarning("QBuffer::seek: Invalid pos: %lld", pos); return false; } return QIODevice::seek(pos); diff --git a/tests/auto/corelib/io/qbuffer/tst_qbuffer.cpp b/tests/auto/corelib/io/qbuffer/tst_qbuffer.cpp index fb756d9e63..a4f26e2c6f 100644 --- a/tests/auto/corelib/io/qbuffer/tst_qbuffer.cpp +++ b/tests/auto/corelib/io/qbuffer/tst_qbuffer.cpp @@ -46,6 +46,7 @@ private slots: void writeBlock_data(); void writeBlock(); void seek(); + void invalidSeeks(); void seekTest_data(); void seekTest(); void read_rawdata(); @@ -291,6 +292,29 @@ void tst_QBuffer::seek() QCOMPARE(buffer.size(), pos); } +void tst_QBuffer::invalidSeeks() +{ + if constexpr (sizeof(qsizetype) == sizeof(qint64)) { + // sizeof(qsizetype) == sizeof(qint64), so +1 would overflow + QSKIP("This is a 32-bit-only test."); + } else { + QBuffer buffer; + buffer.open(QIODevice::WriteOnly); + QCOMPARE(buffer.buffer().size(), qsizetype(0)); + QCOMPARE(buffer.pos(), qint64(0)); + constexpr qint64 MaxQByteArrayCapacity = (std::numeric_limits<qsizetype>::max)(); + // this should fail fast, not after trying to allocate nearly 2 GiB of data, + // potentially crashing in the process: + QVERIFY(!buffer.seek(2 * MaxQByteArrayCapacity - 1)); + QCOMPARE(buffer.buffer().size(), qsizetype(0)); + QCOMPARE(buffer.pos(), qint64(0)); + // ditto: + QVERIFY(!buffer.seek(MaxQByteArrayCapacity + 1)); + QCOMPARE(buffer.buffer().size(), qsizetype(0)); + QCOMPARE(buffer.pos(), qint64(0)); + } +} + void tst_QBuffer::seekTest_data() { writeBlock_data(); |