Bug 1316307 - Fix coverity issues r=ekr
authorTim Taubert <ttaubert@mozilla.com>
Mon, 21 Nov 2016 21:55:28 +0100
changeset 12891 460a0a1e009fe7924d0b3b404783ed0e78381bdf
parent 12890 ea6c6b1734d6bd885c978d357652d037ea67eda0
child 12892 a396de3522078bbde91295479708b4fb43663f4a
push id1807
push userttaubert@mozilla.com
push dateMon, 21 Nov 2016 20:56:33 +0000
reviewersekr
bugs1316307
Bug 1316307 - Fix coverity issues r=ekr Differential Revision: https://nss-review.dev.mozaws.net/D46
gtests/ssl_gtest/databuffer.h
lib/ssl/tls13con.c
lib/ssl/tls13exthandle.c
--- a/gtests/ssl_gtest/databuffer.h
+++ b/gtests/ssl_gtest/databuffer.h
@@ -131,17 +131,17 @@ class DataBuffer {
     len_ = index + ins_len + tail_len;
     data_ = new uint8_t[len_ ? len_ : 1];
 
     // The head of the old.
     if (old_value) {
       Write(0, old_value, std::min(old_len, index));
     }
     // Maybe a gap.
-    if (index > old_len) {
+    if (old_value && index > old_len) {
       memset(old_value + index, 0, index - old_len);
     }
     // The new.
     Write(index, ins, ins_len);
     // The tail of the old.
     if (tail_len > 0) {
       Write(index + ins_len, old_value + index + remove, tail_len);
     }
--- a/lib/ssl/tls13con.c
+++ b/lib/ssl/tls13con.c
@@ -380,16 +380,17 @@ SSL_SendAdditionalKeyShares(PRFileDesc *
  * the requisite type and creates a share for that.
  *
  * Called from ssl3_SendClientHello.
  */
 SECStatus
 tls13_SetupClientHello(sslSocket *ss)
 {
     unsigned int i;
+    SSL3Statistics *ssl3stats = SSL_GetStatistics();
     NewSessionTicket *session_ticket = NULL;
     sslSessionID *sid = ss->sec.ci.sid;
     unsigned int numShares = 0;
 
     PORT_Assert(ss->opt.noLocks || ssl_HaveSSL3HandshakeLock(ss));
     PORT_Assert(ss->opt.noLocks || ssl_HaveXmitBufLock(ss));
     PORT_Assert(PR_CLIST_IS_EMPTY(&ss->ephemeralKeyPairs));
 
@@ -430,25 +431,32 @@ tls13_SetupClientHello(sslSocket *ss)
              session_ticket->received_timestamp >
          ssl_Time())) {
         ss->statelessResume = PR_TRUE;
     }
 
     if (ss->statelessResume) {
         SECStatus rv;
 
+        PORT_Assert(ss->sec.ci.sid);
         rv = tls13_RecoverWrappedSharedSecret(ss, ss->sec.ci.sid);
         if (rv != SECSuccess) {
             FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error);
+            SSL_AtomicIncrementLong(&ssl3stats->sch_sid_cache_not_ok);
+            ss->sec.uncache(ss->sec.ci.sid);
+            ssl_FreeSID(ss->sec.ci.sid);
+            ss->sec.ci.sid = NULL;
             return SECFailure;
         }
 
         rv = ssl3_SetCipherSuite(ss, ss->sec.ci.sid->u.ssl3.cipherSuite, PR_FALSE);
-        if (rv != SECSuccess)
+        if (rv != SECSuccess) {
+            FATAL_ERROR(ss, PORT_GetError(), internal_error);
             return SECFailure;
+        }
 
         rv = tls13_ComputeEarlySecrets(ss);
         if (rv != SECSuccess) {
             FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error);
             return SECFailure;
         }
     }
 
@@ -1329,16 +1337,17 @@ tls13_HandleClientHelloPart2(sslSocket *
                                 &ss->ssl3.hs.srvVirtName) != SECEqual) {
             FATAL_ERROR(ss, SSL_ERROR_RX_MALFORMED_CLIENT_HELLO,
                         handshake_failure);
             goto loser;
         }
 
         rv = tls13_RecoverWrappedSharedSecret(ss, sid);
         if (rv != SECSuccess) {
+            SSL_AtomicIncrementLong(&ssl3stats->hch_sid_cache_not_ok);
             FATAL_ERROR(ss, SEC_ERROR_LIBRARY_FAILURE, internal_error);
             goto loser;
         }
         tls13_RestoreCipherInfo(ss, sid);
 
         ss->sec.serverCert = ssl_FindServerCert(ss, &sid->certType);
         PORT_Assert(ss->sec.serverCert);
         ss->sec.localCert = CERT_DupCertificate(ss->sec.serverCert->serverCert);
@@ -1994,29 +2003,17 @@ tls13_HandleServerHelloPart2(sslSocket *
     }
 
     /* Now create a synthetic kea_def that we can tweak. */
     ss->ssl3.hs.kea_def_mutable = *ss->ssl3.hs.kea_def;
     ss->ssl3.hs.kea_def = &ss->ssl3.hs.kea_def_mutable;
 
     if (ss->statelessResume) {
         /* PSK */
-        PRBool cacheOK = PR_FALSE;
-        do {
-            ss->ssl3.hs.kea_def_mutable.authKeyType = ssl_auth_psk;
-
-            cacheOK = PR_TRUE;
-        } while (0);
-
-        if (!cacheOK) {
-            SSL_AtomicIncrementLong(&ssl3stats->hsh_sid_cache_not_ok);
-            ss->sec.uncache(sid);
-            return SECFailure;
-        }
-
+        ss->ssl3.hs.kea_def_mutable.authKeyType = ssl_auth_psk;
         tls13_RestoreCipherInfo(ss, sid);
         if (sid->peerCert) {
             ss->sec.peerCert = CERT_DupCertificate(sid->peerCert);
         }
 
         SSL_AtomicIncrementLong(&ssl3stats->hsh_sid_cache_hits);
         SSL_AtomicIncrementLong(&ssl3stats->hsh_sid_stateless_resumes);
     } else {
--- a/lib/ssl/tls13exthandle.c
+++ b/lib/ssl/tls13exthandle.c
@@ -528,20 +528,18 @@ tls13_ClientSendPreSharedKeyXtn(const ss
         rv = ssl3_ExtAppendHandshakeVariable(ss,
                                              binder, binderLen, 1);
         if (rv != SECSuccess)
             goto loser;
 
         PRINT_BUF(50, (ss, "Sending PreSharedKey value",
                        session_ticket->ticket.data,
                        session_ticket->ticket.len));
+
         xtnData->sentSessionTicketInClientHello = PR_TRUE;
-        if (rv != SECSuccess)
-            goto loser;
-
         xtnData->advertised[xtnData->numAdvertised++] =
             ssl_tls13_pre_shared_key_xtn;
     }
     return extension_length;
 
 loser:
     xtnData->ticketTimestampVerified = PR_FALSE;
     return -1;