From b4ab4868bc8422e09590d13d35c2bba7a195ccf5 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Wed, 6 Jan 2016 12:58:56 +0100
Subject: Fix UB in QFSFileEnginePrivate::writeFdFh()

Apparently, it is considered valid to call the function with
'data' set to nullptr, and 'len' to zero. But doing so
invokes undefined behavior because nullptr is passed to
fwrite().

Fix by protecting the loops with 'if (len)'.

Found by UBSan:
  qtbase/src/corelib/io/qfsfileengine.cpp:732:84: runtime error: null pointer passed as argument 1, which is declared to never be null

Change-Id: Idfe23875c868ebb21d2164550de3304d2f01e9df
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/io/qfsfileengine.cpp | 46 ++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

(limited to 'src/corelib/io')

diff --git a/src/corelib/io/qfsfileengine.cpp b/src/corelib/io/qfsfileengine.cpp
index 429c40da1a..10e116a23a 100644
--- a/src/corelib/io/qfsfileengine.cpp
+++ b/src/corelib/io/qfsfileengine.cpp
@@ -724,29 +724,33 @@ qint64 QFSFileEnginePrivate::writeFdFh(const char *data, qint64 len)
 
     qint64 writtenBytes = 0;
 
-    if (fh) {
-        // Buffered stdlib mode.
-
-        size_t result;
-        do {
-            result = fwrite(data + writtenBytes, 1, size_t(len - writtenBytes), fh);
-            writtenBytes += result;
-        } while (result == 0 ? errno == EINTR : writtenBytes < len);
+    if (len) { // avoid passing nullptr to fwrite() or QT_WRITE() (UB)
 
-    } else if (fd != -1) {
-        // Unbuffered stdio mode.
+        if (fh) {
+            // Buffered stdlib mode.
+
+            size_t result;
+            do {
+                result = fwrite(data + writtenBytes, 1, size_t(len - writtenBytes), fh);
+                writtenBytes += result;
+            } while (result == 0 ? errno == EINTR : writtenBytes < len);
+
+        } else if (fd != -1) {
+            // Unbuffered stdio mode.
+
+            SignedIOType result;
+            do {
+                // calculate the chunk size
+                // on Windows or 32-bit no-largefile Unix, we'll need to read in chunks
+                // we limit to the size of the signed type, otherwise we could get a negative number as a result
+                quint64 wantedBytes = quint64(len) - quint64(writtenBytes);
+                UnsignedIOType chunkSize = std::numeric_limits<SignedIOType>::max();
+                if (chunkSize > wantedBytes)
+                    chunkSize = wantedBytes;
+                result = QT_WRITE(fd, data + writtenBytes, chunkSize);
+            } while (result > 0 && (writtenBytes += result) < len);
+        }
 
-        SignedIOType result;
-        do {
-            // calculate the chunk size
-            // on Windows or 32-bit no-largefile Unix, we'll need to read in chunks
-            // we limit to the size of the signed type, otherwise we could get a negative number as a result
-            quint64 wantedBytes = quint64(len) - quint64(writtenBytes);
-            UnsignedIOType chunkSize = std::numeric_limits<SignedIOType>::max();
-            if (chunkSize > wantedBytes)
-                chunkSize = wantedBytes;
-            result = QT_WRITE(fd, data + writtenBytes, chunkSize);
-        } while (result > 0 && (writtenBytes += result) < len);
     }
 
     if (len &&  writtenBytes == 0) {
-- 
cgit v1.2.3