Bug 1257885 - Fix some unchecked return values, r=ttaubert
authorFranziskus Kiefer <franziskuskiefer@gmail.com>
Fri, 29 Apr 2016 21:57:47 +0200
changeset 12112 4ebc95c6d748864fe3c5be77d519fbd4f01531a9
parent 12111 db48ed00ec5933684c4fe6482ad8a4e270412afe
child 12113 b3889c64bd61d92d3ad8a40b2d7eec632e16de89
push id1171
push userfranziskuskiefer@gmail.com
push dateFri, 29 Apr 2016 19:59:00 +0000
reviewersttaubert
bugs1257885
Bug 1257885 - Fix some unchecked return values, r=ttaubert
cmd/bltest/blapitest.c
cmd/crlutil/crlutil.c
lib/pkcs12/p12dec.c
lib/smime/cmsrecinfo.c
lib/smime/cmssiginfo.c
--- a/cmd/bltest/blapitest.c
+++ b/cmd/bltest/blapitest.c
@@ -977,16 +977,19 @@ setupIO(PLArenaPool *arena, bltestIO *in
                 input->buf.len = 0;
                 break;
             }
             if (in->data[in->len - 1] == '\n')
                 --in->len;
             if (in->data[in->len - 1] == '\r')
                 --in->len;
             SECITEM_CopyItem(arena, &input->buf, in);
+            if (rv != SECSuccess) {
+                return SECFailure;
+            }
             break;
         case bltestHexSpaceDelim:
             SECITEM_AllocItem(arena, &input->buf, in->len / 5);
             for (i = 0, j = 0; i < in->len; i += 5, j++) {
                 tok = &in->data[i];
                 if (tok[0] != '0' || tok[1] != 'x' || tok[4] != ' ')
                     /* bad hex token */
                     break;
@@ -1056,26 +1059,30 @@ finishIO(bltestIO *output, PRFileDesc *f
                 nb = PR_Write(file, hexstr, 2);
             }
             PR_Write(file, "\n", 1);
             break;
     }
     return rv;
 }
 
-void
+SECStatus
 bltestCopyIO(PLArenaPool *arena, bltestIO *dest, bltestIO *src)
 {
-    SECITEM_CopyItem(arena, &dest->buf, &src->buf);
+    if (SECITEM_CopyItem(arena, &dest->buf, &src->buf) != SECSuccess) {
+        return SECFailure;
+    }
     if (src->pBuf.len > 0) {
         dest->pBuf.len = src->pBuf.len;
         dest->pBuf.data = dest->buf.data + (src->pBuf.data - src->buf.data);
     }
     dest->mode = src->mode;
     dest->file = src->file;
+
+    return SECSuccess;
 }
 
 void
 misalignBuffer(PLArenaPool *arena, bltestIO *io, int off)
 {
     ptrdiff_t offset = (ptrdiff_t)io->buf.data % WORDSIZE;
     int length = io->buf.len;
     if (offset != off) {
@@ -3233,17 +3240,17 @@ blapi_selftest(bltestCipherMode *modes, 
             load_file_data(arena, &ct, filename, bltestBase64Encoded);
 
             get_params(arena, params, mode, j);
             /* Forward Operation (Encrypt/Sign/Hash)
             ** Align the input buffer (plaintext) according to request
             ** then perform operation and compare to ciphertext
             */
             if (encrypt) {
-                bltestCopyIO(arena, &cipherInfo.input, &pt);
+                rv |= bltestCopyIO(arena, &cipherInfo.input, &pt);
                 misalignBuffer(arena, &cipherInfo.input, inoff);
                 memset(&cipherInfo.output.buf, 0, sizeof cipherInfo.output.buf);
                 rv |= cipherInit(&cipherInfo, PR_TRUE);
                 misalignBuffer(arena, &cipherInfo.output, outoff);
                 rv |= cipherDoOp(&cipherInfo);
                 rv |= cipherFinish(&cipherInfo);
                 rv |= verify_self_test(&cipherInfo.output,
                                        &ct, mode, PR_TRUE, SECSuccess);
@@ -3255,30 +3262,30 @@ blapi_selftest(bltestCipherMode *modes, 
                     ** consistency between tests that run Sign/Verify back to
                     ** back (eg: self-tests) and tests that are only running
                     ** verify operations, copy the output into the sig buf,
                     ** and then copy the sig buf back out when verifying. For
                     ** self-tests, this is unnecessary copying, but for
                     ** verify-only operations, this ensures that the output
                     ** buffer is properly configured
                     */
-                    bltestCopyIO(arena, &params->asymk.sig, &cipherInfo.output);
+                    rv |= bltestCopyIO(arena, &params->asymk.sig, &cipherInfo.output);
                 }
             }
             if (!decrypt)
                 continue;
             /* Reverse Operation (Decrypt/Verify)
             ** Align the input buffer (ciphertext) according to request
             ** then perform operation and compare to plaintext
             */
             if (is_sigCipher(mode)) {
-                bltestCopyIO(arena, &cipherInfo.input, &pt);
-                bltestCopyIO(arena, &cipherInfo.output, &params->asymk.sig);
+                rv |= bltestCopyIO(arena, &cipherInfo.input, &pt);
+                rv |= bltestCopyIO(arena, &cipherInfo.output, &params->asymk.sig);
             } else {
-                bltestCopyIO(arena, &cipherInfo.input, &ct);
+                rv |= bltestCopyIO(arena, &cipherInfo.input, &ct);
                 memset(&cipherInfo.output.buf, 0, sizeof cipherInfo.output.buf);
             }
             misalignBuffer(arena, &cipherInfo.input, inoff);
             rv |= cipherInit(&cipherInfo, PR_FALSE);
             misalignBuffer(arena, &cipherInfo.output, outoff);
             srv = SECSuccess;
             srv |= cipherDoOp(&cipherInfo);
             rv |= cipherFinish(&cipherInfo);
--- a/cmd/crlutil/crlutil.c
+++ b/cmd/crlutil/crlutil.c
@@ -34,32 +34,38 @@ FindCRL(CERTCertDBHandle *certHandle, ch
 
     derName.data = NULL;
     derName.len = 0;
 
     cert = CERT_FindCertByNicknameOrEmailAddr(certHandle, name);
     if (!cert) {
         CERTName *certName = NULL;
         PLArenaPool *arena = NULL;
+        SECStatus rv = SECSuccess;
 
         certName = CERT_AsciiToName(name);
         if (certName) {
             arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
             if (arena) {
                 SECItem *nameItem =
                     SEC_ASN1EncodeItem(arena, NULL, (void *)certName,
                                        SEC_ASN1_GET(CERT_NameTemplate));
                 if (nameItem) {
-                    SECITEM_CopyItem(NULL, &derName, nameItem);
+                    rv = SECITEM_CopyItem(NULL, &derName, nameItem);
                 }
                 PORT_FreeArena(arena, PR_FALSE);
             }
             CERT_DestroyName(certName);
         }
 
+        if (rv != SECSuccess) {
+            SECU_PrintError(progName, "SECITEM_CopyItem failed, out of memory");
+            return ((CERTSignedCrl *)NULL);
+        }
+
         if (!derName.len || !derName.data) {
             SECU_PrintError(progName, "could not find certificate named '%s'", name);
             return ((CERTSignedCrl *)NULL);
         }
     } else {
         SECITEM_CopyItem(NULL, &derName, &cert->derSubject);
         CERT_DestroyCertificate(cert);
     }
--- a/lib/pkcs12/p12dec.c
+++ b/lib/pkcs12/p12dec.c
@@ -57,18 +57,28 @@ sec_pkcs12_decode_pfx(SECItem *der_pfx)
 	rv = SEC_ASN1DecodeItem(pfx->poolp, pfx, SEC_PKCS12PFXItemTemplate_OLD, 
 				der_pfx);
 	if(rv != SECSuccess) {
 	    PORT_SetError(SEC_ERROR_PKCS12_DECODING_PFX);
 	    PORT_FreeArena(pfx->poolp, PR_TRUE);
 	    return NULL;
 	}
 	pfx->old = PR_TRUE;
-	SGN_CopyDigestInfo(pfx->poolp, &pfx->macData.safeMac, &pfx->old_safeMac);
-	SECITEM_CopyItem(pfx->poolp, &pfx->macData.macSalt, &pfx->old_macSalt);
+	rv = SGN_CopyDigestInfo(pfx->poolp, &pfx->macData.safeMac, &pfx->old_safeMac);
+	if(rv != SECSuccess) {
+	    PORT_SetError(SEC_ERROR_NO_MEMORY);
+	    PORT_FreeArena(pfx->poolp, PR_TRUE);
+	    return NULL;
+	}
+	rv = SECITEM_CopyItem(pfx->poolp, &pfx->macData.macSalt, &pfx->old_macSalt);
+	if(rv != SECSuccess) {
+	    PORT_SetError(SEC_ERROR_NO_MEMORY);
+	    PORT_FreeArena(pfx->poolp, PR_TRUE);
+	    return NULL;
+	}
     } else {
 	pfx->old = PR_FALSE;
     }
 
     /* convert bit string from bits to bytes */
     pfx->macData.macSalt.len /= 8;
 
     return pfx;
--- a/lib/smime/cmsrecinfo.c
+++ b/lib/smime/cmsrecinfo.c
@@ -133,18 +133,18 @@ nss_cmsrecipientinfo_create(NSSCMSMessag
 	    NSSCMSKeyTransRecipientInfoEx *riExtra;
 
 	    rid->id.subjectKeyID = PORT_ArenaNew(poolp, SECItem);
 	    if (rid->id.subjectKeyID == NULL) {
 		rv = SECFailure;
 		PORT_SetError(SEC_ERROR_NO_MEMORY);
 		break;
 	    } 
-	    SECITEM_CopyItem(poolp, rid->id.subjectKeyID, subjKeyID);
-	    if (rid->id.subjectKeyID->data == NULL) {
+	    rv = SECITEM_CopyItem(poolp, rid->id.subjectKeyID, subjKeyID);
+	    if (rv != SECSuccess || rid->id.subjectKeyID->data == NULL) {
 		rv = SECFailure;
 		PORT_SetError(SEC_ERROR_NO_MEMORY);
 		break;
 	    }
 	    riExtra = &ri->ri.keyTransRecipientInfoEx;
 	    riExtra->version = 0;
 	    riExtra->pubKey = SECKEY_CopyPublicKey(pubKey);
 	    if (riExtra->pubKey == NULL) {
--- a/lib/smime/cmssiginfo.c
+++ b/lib/smime/cmssiginfo.c
@@ -46,16 +46,17 @@ NSSCMSSignerInfo *
 nss_cmssignerinfo_create(NSSCMSMessage *cmsg, NSSCMSSignerIDSelector type, 
 	CERTCertificate *cert, SECItem *subjKeyID, SECKEYPublicKey *pubKey, 
 	SECKEYPrivateKey *signingKey, SECOidTag digestalgtag)
 {
     void *mark;
     NSSCMSSignerInfo *signerinfo;
     int version;
     PLArenaPool *poolp;
+    SECStatus rv;
 
     poolp = cmsg->poolp;
 
     mark = PORT_ArenaMark(poolp);
 
     signerinfo = (NSSCMSSignerInfo *)PORT_ArenaZAlloc(poolp, sizeof(NSSCMSSignerInfo));
     if (signerinfo == NULL) {
 	PORT_ArenaRelease(poolp, mark);
@@ -75,18 +76,21 @@ nss_cmssignerinfo_create(NSSCMSMessage *
         break;
     case NSSCMSSignerID_SubjectKeyID:
         signerinfo->signerIdentifier.identifierType = NSSCMSSignerID_SubjectKeyID;
         PORT_Assert(subjKeyID);
         if (!subjKeyID)
             goto loser;
 
         signerinfo->signerIdentifier.id.subjectKeyID = PORT_ArenaNew(poolp, SECItem);
-        SECITEM_CopyItem(poolp, signerinfo->signerIdentifier.id.subjectKeyID,
-                         subjKeyID);
+        rv = SECITEM_CopyItem(poolp, signerinfo->signerIdentifier.id.subjectKeyID,
+                              subjKeyID);
+        if (rv != SECSuccess) {
+            goto loser;
+        }
         signerinfo->signingKey = SECKEY_CopyPrivateKey(signingKey);
         if (!signerinfo->signingKey)
             goto loser;
         signerinfo->pubKey = SECKEY_CopyPublicKey(pubKey);
         if (!signerinfo->pubKey)
             goto loser;
         break;
     default: