Fix a buffer length bug and nits in the next protocol negotiation (NPN)

net/third_party/nss/ssl/ssl3ext.c

 /*
  * SSL3 Protocol
  *
  * ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
  * The contents of this file are subject to the Mozilla Public License Version
  * 1.1 (the "License"); you may not use this file except in compliance with
  * the License. You may obtain a copy of the License at
  * http://www.mozilla.org/MPL/
@@ skipping 588 matching lines @@
 }
 
 static SECStatus
 ssl3_ClientHandleNextProtoNegoXtn(sslSocket *ss, PRUint16 ex_type,
                                   SECItem *data)
 {
     SECStatus rv;
     unsigned char resultBuffer[255];
     SECItem result = { siBuffer, resultBuffer, 0 };
 
-    if (ss->firstHsDone) {

[wtc, 2012/03/10 00:43:15]

ss->firstHsDone cannot be true here.  Line 650 below ensures
that we (an NSS client) do not send the NPN extension if
ss->firstHsDone is true, and NSS won't call an extension
handler on the client side if the server sends an unadvertised
extension.

-        PORT_SetError(SSL_ERROR_NEXT_PROTOCOL_DATA_INVALID);
-        return SECFailure;
-    }
+    PORT_Assert(!ss->firstHsDone);
 
     rv = ssl3_ValidateNextProtoNego(data->data, data->len);
     if (rv != SECSuccess)
         return rv;
 
     /* ss->nextProtoCallback cannot normally be NULL if we negotiated the
      * extension. However, It is possible that an application erroneously
      * cleared the callback between the time we sent the ClientHello and now.
      */
     PORT_Assert(ss->nextProtoCallback != NULL);
     if (!ss->nextProtoCallback) {
+        /* XXX Use a better error code. This is an application error, not an
+         * NSS bug. */
         PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
         return SECFailure;
     }
 
     rv = ss->nextProtoCallback(ss->nextProtoArg, ss->fd, data->data, data->len,
                                result.data, &result.len, sizeof resultBuffer);
     if (rv != SECSuccess)
         return rv;
     /* If the callback wrote more than allowed to |result| it has corrupted our
      * stack. */
-    if (result.len > sizeof result) {
+    if (result.len > sizeof resultBuffer) {

[wtc, 2012/03/10 00:43:15]

This is the fix for the buffer length bug.  We will hit this
bug if the NPN result is larger than the size of SECItem
(12 bytes).

         PORT_SetError(SEC_ERROR_OUTPUT_LEN);
         return SECFailure;
     }
 
     SECITEM_FreeItem(&ss->ssl3.nextProto, PR_FALSE);
     return SECITEM_CopyItem(NULL, &ss->ssl3.nextProto, &result);
 }
 
 static PRInt32
 ssl3_ClientSendNextProtoNegoXtn(sslSocket * ss, PRBool append,
@@ skipping 1217 matching lines @@
 
     /* The echoed extension must be empty. */
     if (data->len != 0)
         return SECFailure;
 
     /* Keep track of negotiated extensions. */
     ss->xtnData.negotiated[ss->xtnData.numNegotiated++] = ex_type;
 
     return SECSuccess;
 }