Fix freeze and high CPU usage on invalid STDIN data, resolves #1620

keepassxreboot · Mar 6, 2018 · e78fb78 · e78fb78
1 parent 386b78b
commit e78fb78
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 23 deletions.
diff --git a/src/browser/NativeMessagingHost.cpp b/src/browser/NativeMessagingHost.cpp
@@ -107,18 +107,26 @@ void NativeMessagingHost::readLength()
 
 void NativeMessagingHost::readStdIn(const quint32 length)
 {
-    if (length > 0) {
-        QByteArray arr;
-        arr.reserve(length);
+    if (length <= 0) {
+        return;
+    }
 
-        for (quint32 i = 0; i < length; ++i) {
-            arr.append(getchar());
-        }
+    QByteArray arr;
+    arr.reserve(length);
 
-        if (arr.length() > 0) {
-            QMutexLocker locker(&m_mutex);
-            sendReply(m_browserClients.readResponse(arr));
+    QMutexLocker locker(&m_mutex);
+
+    for (quint32 i = 0; i < length; ++i) {
+        int c = std::getchar();
+        if (c == EOF) {
+            // message ended prematurely, ignore it and return
+            return;
         }
+        arr.append(static_cast<char>(c));
+    }
+
+    if (arr.length() > 0) {
+        sendReply(m_browserClients.readResponse(arr));
     }
 }
 

diff --git a/src/main.cpp b/src/main.cpp
@@ -143,12 +143,15 @@ int main(int argc, char** argv)
 
     const bool pwstdin = parser.isSet(pwstdinOption);
     for (const QString& filename: fileNames) {
+        QString password;
+        if (pwstdin) {
+            // we always need consume a line of STDIN if --pw-stdin is set to clear out the
+            // buffer for native messaging, even if the specified file does not exist
+            static QTextStream in(stdin, QIODevice::ReadOnly);
+            password = in.readLine();
+        }
+
         if (!filename.isEmpty() && QFile::exists(filename) && !filename.endsWith(".json", Qt::CaseInsensitive)) {
-            QString password;
-            if (pwstdin) {
-                static QTextStream in(stdin, QIODevice::ReadOnly);
-                password = in.readLine();
-            }
             mainWindow.openDatabase(filename, password, parser.value(keyfileOption));
         }
     }

diff --git a/src/proxy/NativeMessagingHost.cpp b/src/proxy/NativeMessagingHost.cpp
@@ -51,18 +51,25 @@ void NativeMessagingHost::readLength()
 
 void NativeMessagingHost::readStdIn(const quint32 length)
 {
-    if (length > 0) {
-        QByteArray arr;
-        arr.reserve(length);
+    if (length <= 0) {
+        return;
+    }
 
-        for (quint32 i = 0; i < length; ++i) {
-            arr.append(getchar());
-        }
+    QByteArray arr;
+    arr.reserve(length);
 
-        if (arr.length() > 0 && m_localSocket && m_localSocket->state() == QLocalSocket::ConnectedState) {
-            m_localSocket->write(arr.constData(), arr.length());
-            m_localSocket->flush();
+    for (quint32 i = 0; i < length; ++i) {
+        int c = std::getchar();
+        if (c == EOF) {
+            // message ended prematurely, ignore it and return
+            return;
         }
+        arr.append(static_cast<char>(c));
+    }
+
+    if (arr.length() > 0 && m_localSocket && m_localSocket->state() == QLocalSocket::ConnectedState) {
+        m_localSocket->write(arr.constData(), arr.length());
+        m_localSocket->flush();
     }
 }