Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-20452

Using two different QSslSockets in two different threads will crash because of const-corectness issues in QSslCertificate

    XMLWordPrintable

Details

    • Bug
    • Resolution: Done
    • P2: Important
    • 4.8.1, 5.0.0
    • 4.8.0
    • Network: SSL
    • None
    • 656fab5e848fd14e5d00536a4babbb2f33dbcfb7 (4.8) 00821ec710ec8d9549b9f7e68837a7393a954754 (5.0)

    Description

      1. Suppose the app developer has two threads with two different instances of QSSlSocket

      2. Each thread calls QSslSocket::startClientEncryption(). So far so good.

      3. Inside startClientEncryption(), further down the call chain, at line 332 in qsslsocket_openssl.cpp, we iterate over the certificates returned from QSSlSocket::caCertificates(), and we check if they are valid. Those certificates are shared between instances, but since QSslCertificate::isValid() is supposed to be a const operation, there should be no harm in doing this.

      The problem is: QSslCertificate::isValid() actually mutates itself

      Why?
      Because after the Comodo incident back in March, QSslCertificate::isValid() now calls isBlacklisted() wich calls serialNumber() which calculates, stores, and returns the serial number. This is a non-const operation on the d-pointer, but since it happens in a const method, the d-pointer is not being detached, ultimately causing a crash since the other thread is doing the same thing.

      This is a problem with many methods of QSslCertificate that are marked as const but actually aren't since they use this calculate-and-store-result-on-first-use idiom. If they where using the Q_D macro, this would have been caught by the compiler as a const-corectness error.

      Attachments

        Issue Links

          For Gerrit Dashboard: QTBUG-20452
          # Subject Branch Project Status CR V

          Activity

            People

              shkearns Shane Kearns
              gregschlom Gregory Schlomoff
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Gerrit Reviews

                  There are no open Gerrit changes