Hello, we've run a bunch of checkers against the latest xmlsec code and I have a few patches you may want to consider for inclusion.
Some are no brainers, others you may like or not. I would like to apply them downstream once they are accepted in master, let me know what you think. Simo. -- Simo Sorce * Red Hat, Inc * New York
>From e751424380b52b64dfdda58b7811bbd21103e3d5 Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Tue, 3 Jun 2014 13:32:22 -0400 Subject: [PATCH 1/7] Check returned value Coverity complains some returned values are not being checked. This patch implements those checks. Signed-off-by: Simo Sorce <[email protected]> --- src/keyinfo.c | 11 ++++++++++- src/xmldsig.c | 14 ++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/keyinfo.c b/src/keyinfo.c index 00390fa7003ff7099bac4feefa056280b02f7b45..61ef2bb102e31345c9ccdd89b76520152697dcf9 100644 --- a/src/keyinfo.c +++ b/src/keyinfo.c @@ -761,7 +761,16 @@ xmlSecKeyDataNameXmlRead(xmlSecKeyDataId id, xmlSecKeyPtr key, xmlNodePtr node, /* finally set key name if it is not there */ if(xmlSecKeyGetName(key) == NULL) { - xmlSecKeySetName(key, newName); + ret = xmlSecKeySetName(key, newName); + if(ret < 0) { + xmlSecError(XMLSEC_ERRORS_HERE, + xmlSecErrorsSafeString(xmlSecKeyDataKlassGetName(id)), + "xmlSecKeySetName", + XMLSEC_ERRORS_R_XMLSEC_FAILED, + XMLSEC_ERRORS_NO_MESSAGE); + xmlFree(newName); + return(-1); + } } xmlFree(newName); return(0); diff --git a/src/xmldsig.c b/src/xmldsig.c index b08b8b119b7520257ca752c815e06d6d10e51492..b37975fe9165f09fa9f4049c47ca90211f76a336 100644 --- a/src/xmldsig.c +++ b/src/xmldsig.c @@ -160,10 +160,16 @@ xmlSecDSigCtxInitialize(xmlSecDSigCtxPtr dsigCtx, xmlSecKeysMngrPtr keysMngr) { } /* references lists from SignedInfo and Manifest elements */ - xmlSecPtrListInitialize(&(dsigCtx->signedInfoReferences), - xmlSecDSigReferenceCtxListId); - xmlSecPtrListInitialize(&(dsigCtx->manifestReferences), - xmlSecDSigReferenceCtxListId); + ret = xmlSecPtrListInitialize(&(dsigCtx->signedInfoReferences), + xmlSecDSigReferenceCtxListId); + if(ret != 0) { + return(ret); + } + ret = xmlSecPtrListInitialize(&(dsigCtx->manifestReferences), + xmlSecDSigReferenceCtxListId); + if(ret != 0) { + return(ret); + } dsigCtx->enabledReferenceUris = xmlSecTransformUriTypeAny; return(0); -- 1.9.3
>From 310971ea5736f14c3fbe9ccb9496a0335e1fe0e2 Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Tue, 3 Jun 2014 15:01:28 -0400 Subject: [PATCH 2/7] Fix dead code Coverity marked these 2 locations as dead code. The code in transforms.c is effectively dead as buf is initialized to NULL and never set if useVisa3DHack != 0 The second is dead only in the sense that the assert is being invoked incorrectly, in order for it to fire the condition need to be false. Signed-off-by: Simo Sorce <[email protected]> --- src/openssl/kt_rsa.c | 2 +- src/transforms.c | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/openssl/kt_rsa.c b/src/openssl/kt_rsa.c index 1cf1aba1bfb4ae75e596bbb846c46f6871cd45fd..a7afcc367e94c15f5bf1486f58704ab313bf8974 100644 --- a/src/openssl/kt_rsa.c +++ b/src/openssl/kt_rsa.c @@ -845,7 +845,7 @@ xmlSecOpenSSLRsaOaepProcess(xmlSecTransformPtr transform, xmlSecTransformCtxPtr } outSize = ret; } else { - xmlSecAssert2("we could not be here" == NULL, -1); + xmlSecAssert2("we could not be here" != NULL, -1); return(-1); } diff --git a/src/transforms.c b/src/transforms.c index 8a2ded236a14911f39bab6e31af877ed360debfd..0455973bb0942a3034eb39c2d597984536fb6e7e 100644 --- a/src/transforms.c +++ b/src/transforms.c @@ -976,9 +976,6 @@ xmlSecTransformCtxSetUri(xmlSecTransformCtxPtr ctx, const xmlChar* uri, xmlNodeP XMLSEC_ERRORS_R_XMLSEC_FAILED, "name=%s", xmlSecErrorsSafeString(xmlSecTransformGetName(transform))); - if(buf != NULL) { - xmlFree(buf); - } return(-1); } } -- 1.9.3
>From a84b2a6fe9d2a6af325c545fda49342220d12bc6 Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Tue, 3 Jun 2014 15:07:52 -0400 Subject: [PATCH 3/7] Fix potential NULL pointer dereference Coeverity complains that previous tests indicate that cur could be NULL. Check cur is not null before tryng to dereference it. Signed-off-by: Simo Sorce <[email protected]> --- src/xmldsig.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/xmldsig.c b/src/xmldsig.c index b37975fe9165f09fa9f4049c47ca90211f76a336..8d9e2dce1621afaa370dbfc0c621c617d494259b 100644 --- a/src/xmldsig.c +++ b/src/xmldsig.c @@ -779,7 +779,9 @@ xmlSecDSigCtxProcessSignedInfoNode(xmlSecDSigCtxPtr dsigCtx, xmlNodePtr node) { dsigCtx->signMethod->operation = dsigCtx->operation; /* calculate references */ - cur = xmlSecGetNextElementNode(cur->next); + if(cur) { + cur = xmlSecGetNextElementNode(cur->next); + } while((cur != NULL) && (xmlSecCheckNodeName(cur, xmlSecNodeReference, xmlSecDSigNs))) { /* create reference */ dsigRefCtx = xmlSecDSigReferenceCtxCreate(dsigCtx, xmlSecDSigReferenceOriginSignedInfo); -- 1.9.3
>From 5f54839b83df93958f9eb3de7d458ad5fc225d93 Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Tue, 3 Jun 2014 15:28:06 -0400 Subject: [PATCH 4/7] Fix printf format warnings The compiler complains that %d (a 4 byte quantity on x86_64) is used, but the argument is an 8 byte quantity (long or size_t). Signed-off-by: Simo Sorce <[email protected]> --- apps/cmdline.c | 4 ++-- src/base64.c | 2 +- src/buffer.c | 2 +- src/dl.c | 2 +- src/io.c | 2 +- src/keyinfo.c | 2 +- src/keys.c | 4 ++-- src/keysmngr.c | 2 +- src/list.c | 4 ++-- src/nodeset.c | 2 +- src/transforms.c | 6 +++--- src/xmldsig.c | 4 ++-- src/xmlenc.c | 2 +- src/xpath.c | 4 ++-- 14 files changed, 21 insertions(+), 21 deletions(-) diff --git a/apps/cmdline.c b/apps/cmdline.c index b9ecafb5e71fb23274450877df09d6c17dcb74cf..eb95d9a9fafd60b7085dc5385eb53bd096083490 100644 --- a/apps/cmdline.c +++ b/apps/cmdline.c @@ -152,7 +152,7 @@ xmlSecAppCmdLineValueCreate(xmlSecAppCmdLineParamPtr param, int pos) { assert(param != NULL); value = (xmlSecAppCmdLineValuePtr) malloc(sizeof(xmlSecAppCmdLineValue)); if(value == NULL) { - fprintf(stderr, "Error: malloc failed (%d bytes).\n", sizeof(xmlSecAppCmdLineValue)); + fprintf(stderr, "Error: malloc failed (%d bytes).\n", (int)sizeof(xmlSecAppCmdLineValue)); return(NULL); } memset(value, 0, sizeof(xmlSecAppCmdLineValue)); @@ -284,7 +284,7 @@ xmlSecAppCmdLineParamRead(xmlSecAppCmdLineParamPtr param, const char** argv, int value->strValue = argv[++pos]; buf = (char*)malloc(strlen(value->strValue) + 2); if(buf == NULL) { - fprintf(stderr, "Error: failed to allocate memory (%d bytes).\n", strlen(value->strValue) + 2); + fprintf(stderr, "Error: failed to allocate memory (%d bytes).\n", (int)strlen(value->strValue) + 2); return(-1); } memset(buf, 0, strlen(value->strValue) + 2); diff --git a/src/base64.c b/src/base64.c index 53e6694580b544099f563f87906509f0de5a1a6d..0546582cdf3db1ceded3226fed514238ed452452 100644 --- a/src/base64.c +++ b/src/base64.c @@ -161,7 +161,7 @@ xmlSecBase64CtxCreate(int encode, int columns) { NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecBase64Ctx)=%d", - sizeof(xmlSecBase64Ctx)); + (int)sizeof(xmlSecBase64Ctx)); return(NULL); } diff --git a/src/buffer.c b/src/buffer.c index 0efbfed2b49c0c2c1cf12fe07e792d1054f7906b..52c5fc94ce4a92221253f8af28e729c8df5e5cfc 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -67,7 +67,7 @@ xmlSecBufferCreate(xmlSecSize size) { NULL, NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, - "sizeof(xmlSecBuffer)=%d", sizeof(xmlSecBuffer)); + "sizeof(xmlSecBuffer)=%d", (int)sizeof(xmlSecBuffer)); return(NULL); } diff --git a/src/dl.c b/src/dl.c index 5ffc2ff77fce1db6d9d7f8a161dd768e62126643..255818f404024ff39ba4aa8262cdbe80660d3fb5 100644 --- a/src/dl.c +++ b/src/dl.c @@ -102,7 +102,7 @@ xmlSecCryptoDLLibraryCreate(const xmlChar* name) { NULL, NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, - "size=%d", sizeof(lib)); + "size=%d", (int)sizeof(lib)); return(NULL); } memset(lib, 0, sizeof(xmlSecCryptoDLLibrary)); diff --git a/src/io.c b/src/io.c index 42e91337bc34734e548b4839c47765e3e6d2e972..3f3b9efec6656f58035a992567604d4bb309432b 100644 --- a/src/io.c +++ b/src/io.c @@ -66,7 +66,7 @@ xmlSecIOCallbackCreate(xmlInputMatchCallback matchFunc, xmlInputOpenCallback ope NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecIOCallback)=%d", - sizeof(xmlSecIOCallback)); + (int)sizeof(xmlSecIOCallback)); return(NULL); } memset(callbacks, 0, sizeof(xmlSecIOCallback)); diff --git a/src/keyinfo.c b/src/keyinfo.c index 61ef2bb102e31345c9ccdd89b76520152697dcf9..7fc6a4bb22fb7790ed6295777437bbedd3b32d0b 100644 --- a/src/keyinfo.c +++ b/src/keyinfo.c @@ -227,7 +227,7 @@ xmlSecKeyInfoCtxCreate(xmlSecKeysMngrPtr keysMngr) { NULL, NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, - "size=%d", sizeof(xmlSecKeyInfoCtx)); + "size=%d", (int)sizeof(xmlSecKeyInfoCtx)); return(NULL); } diff --git a/src/keys.c b/src/keys.c index 1d2f7331cca90cea40d3059c9d1e75ec0349cb09..27f3690a98db2812c8cf276fde4b8d15f282aed3 100644 --- a/src/keys.c +++ b/src/keys.c @@ -112,7 +112,7 @@ xmlSecKeyUseWithCreate(const xmlChar* application, const xmlChar* identifier) { NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecKeyUseWith)=%d", - sizeof(xmlSecKeyUseWith)); + (int)sizeof(xmlSecKeyUseWith)); return(NULL); } memset(keyUseWith, 0, sizeof(xmlSecKeyUseWith)); @@ -548,7 +548,7 @@ xmlSecKeyCreate(void) { NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecKey)=%d", - sizeof(xmlSecKey)); + (int)sizeof(xmlSecKey)); return(NULL); } memset(key, 0, sizeof(xmlSecKey)); diff --git a/src/keysmngr.c b/src/keysmngr.c index 31a03e979a02a439c9a1d10ab944b5b9672ddaa5..ad253c9a143d8c396fc793e3e76c21c5ecdfea54 100644 --- a/src/keysmngr.c +++ b/src/keysmngr.c @@ -53,7 +53,7 @@ xmlSecKeysMngrCreate(void) { NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecKeysMngr)=%d", - sizeof(xmlSecKeysMngr)); + (int)sizeof(xmlSecKeysMngr)); return(NULL); } memset(mngr, 0, sizeof(xmlSecKeysMngr)); diff --git a/src/list.c b/src/list.c index d1a00533f7b8a8a24032a0c9df57f3ecf580e7b3..1d48cc67ba2557a384fec6bbc972ea64b8abe43e 100644 --- a/src/list.c +++ b/src/list.c @@ -65,7 +65,7 @@ xmlSecPtrListCreate(xmlSecPtrListId id) { NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecPtrList)=%d", - sizeof(xmlSecPtrList)); + (int)sizeof(xmlSecPtrList)); return(NULL); } @@ -479,7 +479,7 @@ xmlSecPtrListEnsureSize(xmlSecPtrListPtr list, xmlSecSize size) { NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecPtr)*%d=%d", - newSize, sizeof(xmlSecPtr) * newSize); + newSize, (int)(sizeof(xmlSecPtr) * newSize)); return(-1); } diff --git a/src/nodeset.c b/src/nodeset.c index 04ae8105c93091f4f46f5c5c55246f691be61c2d..fbb3ecd09215913cb40c3b3fb3cba238f5962bd9 100644 --- a/src/nodeset.c +++ b/src/nodeset.c @@ -57,7 +57,7 @@ xmlSecNodeSetCreate(xmlDocPtr doc, xmlNodeSetPtr nodes, xmlSecNodeSetType type) NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecNodeSet)=%d", - sizeof(xmlSecNodeSet)); + (int)sizeof(xmlSecNodeSet)); return(NULL); } memset(nset, 0, sizeof(xmlSecNodeSet)); diff --git a/src/transforms.c b/src/transforms.c index 0455973bb0942a3034eb39c2d597984536fb6e7e..29aa2cfc117d49d4331d6460783b71acc310c6dc 100644 --- a/src/transforms.c +++ b/src/transforms.c @@ -355,7 +355,7 @@ xmlSecTransformCtxCreate(void) { NULL, NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, - "size=%d", sizeof(xmlSecTransformCtx)); + "size=%d", (int)sizeof(xmlSecTransformCtx)); return(NULL); } @@ -876,7 +876,7 @@ xmlSecTransformCtxSetUri(xmlSecTransformCtxPtr ctx, const xmlChar* uri, xmlNodeP NULL, NULL, XMLSEC_ERRORS_R_STRDUP_FAILED, - "size=%d", xptr - uri); + "size=%d", (int)(xptr - uri)); return(-1); } @@ -2807,7 +2807,7 @@ xmlSecTransformIOBufferCreate(xmlSecTransformIOBufferMode mode, xmlSecTransformP NULL, NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, - "size=%d", sizeof(xmlSecTransformIOBuffer)); + "size=%d", (int)sizeof(xmlSecTransformIOBuffer)); return(NULL); } memset(buffer, 0, sizeof(xmlSecTransformIOBuffer)); diff --git a/src/xmldsig.c b/src/xmldsig.c index 8d9e2dce1621afaa370dbfc0c621c617d494259b..ea4797fe90506fd52cc4fde9c5bc368de1ff4139 100644 --- a/src/xmldsig.c +++ b/src/xmldsig.c @@ -73,7 +73,7 @@ xmlSecDSigCtxCreate(xmlSecKeysMngrPtr keysMngr) { NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecDSigCtx)=%d", - sizeof(xmlSecDSigCtx)); + (int)sizeof(xmlSecDSigCtx)); return(NULL); } @@ -1268,7 +1268,7 @@ xmlSecDSigReferenceCtxCreate(xmlSecDSigCtxPtr dsigCtx, xmlSecDSigReferenceOrigin NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecDSigReferenceCtx)=%d", - sizeof(xmlSecDSigReferenceCtx)); + (int)sizeof(xmlSecDSigReferenceCtx)); return(NULL); } diff --git a/src/xmlenc.c b/src/xmlenc.c index 44c98779d3c4643a4fb5eb8f59476ca91f7a106b..04801595d093b0ed2aa72cc09a27abf8494b53e1 100644 --- a/src/xmlenc.c +++ b/src/xmlenc.c @@ -65,7 +65,7 @@ xmlSecEncCtxCreate(xmlSecKeysMngrPtr keysMngr) { NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecEncCtx)=%d", - sizeof(xmlSecEncCtx)); + (int)sizeof(xmlSecEncCtx)); return(NULL); } diff --git a/src/xpath.c b/src/xpath.c index e67631e761d59b4a8c6939845a159e24f1386f3e..34ce28556a7e7b6b38550b1f712e289fd1a94232 100644 --- a/src/xpath.c +++ b/src/xpath.c @@ -91,7 +91,7 @@ xmlSecXPathDataCreate(xmlSecXPathDataType type) { NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "sizeof(xmlSecXPathData)=%d", - sizeof(xmlSecXPathData)); + (int)sizeof(xmlSecXPathData)); return(NULL); } memset(data, 0, sizeof(xmlSecXPathData)); @@ -613,7 +613,7 @@ xmlSecTransformXPathNodeRead(xmlSecTransformPtr transform, xmlNodePtr node, xmlS NULL, XMLSEC_ERRORS_R_MALLOC_FAILED, "size=%d", - xmlStrlen(data->expr) + strlen(xpathPattern) + 1); + (int)(xmlStrlen(data->expr) + strlen(xpathPattern) + 1)); return(-1); } sprintf((char*)tmp, xpathPattern, (char*)data->expr); -- 1.9.3
>From 0b20f9306ab8376a019b3b37f86c9a8f4d459575 Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Tue, 3 Jun 2014 16:44:48 -0400 Subject: [PATCH 5/7] Silence warnings about unused computed values Signed-off-by: Simo Sorce <[email protected]> --- apps/xmlsec.c | 2 +- src/openssl/app.c | 4 ++-- src/openssl/x509.c | 6 +++--- src/xmldsig.c | 48 ++++++++++++++++++++++++------------------------ src/xmlenc.c | 12 ++++++------ 5 files changed, 36 insertions(+), 36 deletions(-) diff --git a/apps/xmlsec.c b/apps/xmlsec.c index c2f3196735cb1273ba3c9bf0c811f5d09790b3dd..c9e553422efce302e4b594c99b58856417d35101 100644 --- a/apps/xmlsec.c +++ b/apps/xmlsec.c @@ -2986,7 +2986,7 @@ xmlSecAppWriteResult(xmlDocPtr doc, xmlSecBufferPtr buffer) { if(doc != NULL) { xmlDocDump(f, doc); } else if((buffer != NULL) && (xmlSecBufferGetData(buffer) != NULL)) { - fwrite(xmlSecBufferGetData(buffer), xmlSecBufferGetSize(buffer), 1, f); + (void)fwrite(xmlSecBufferGetData(buffer), xmlSecBufferGetSize(buffer), 1, f); } else { fprintf(stderr, "Error: both result doc and result buffer are null\n"); xmlSecAppCloseFile(f); diff --git a/src/openssl/app.c b/src/openssl/app.c index 4f8f79e66752ec6dc2fd5036f7d83c5803eccbca..4154d2ef6ab3339adfa99880ff46df78b28e0f00 100644 --- a/src/openssl/app.c +++ b/src/openssl/app.c @@ -255,7 +255,7 @@ xmlSecOpenSSLAppKeyLoadBIO(BIO* bio, xmlSecKeyDataFormat format, } if(pKey == NULL) { /* go to start of the file and try to read public key */ - BIO_reset(bio); + (void)BIO_reset(bio); pKey = PEM_read_bio_PUBKEY(bio, NULL, XMLSEC_PTR_TO_FUNC(pem_password_cb, pwdCallback), pwdCallbackCtx); @@ -274,7 +274,7 @@ xmlSecOpenSSLAppKeyLoadBIO(BIO* bio, xmlSecKeyDataFormat format, pKey = d2i_PrivateKey_bio(bio, NULL); if(pKey == NULL) { /* go to start of the file and try to read public key */ - BIO_reset(bio); + (void)BIO_reset(bio); pKey = d2i_PUBKEY_bio(bio, NULL); if(pKey == NULL) { xmlSecError(XMLSEC_ERRORS_HERE, diff --git a/src/openssl/x509.c b/src/openssl/x509.c index 459a312d428a71a108d385b90c90b713ea8c9de4..11f4571cf03c6f9fa7e2d81ad5d87395f484fcf9 100644 --- a/src/openssl/x509.c +++ b/src/openssl/x509.c @@ -1941,7 +1941,7 @@ xmlSecOpenSSLX509CertBase64DerWrite(X509* cert, int base64LineWrap) { /* todo: add error checks */ i2d_X509_bio(mem, cert); - BIO_flush(mem); + (void)BIO_flush(mem); size = BIO_get_mem_data(mem, &p); if((size <= 0) || (p == NULL)){ @@ -2055,7 +2055,7 @@ xmlSecOpenSSLX509CrlBase64DerWrite(X509_CRL* crl, int base64LineWrap) { /* todo: add error checks */ i2d_X509_CRL_bio(mem, crl); - BIO_flush(mem); + (void)BIO_flush(mem); size = BIO_get_mem_data(mem, &p); if((size <= 0) || (p == NULL)){ @@ -2111,7 +2111,7 @@ xmlSecOpenSSLX509NameWrite(X509_NAME* nm) { return(NULL); } - BIO_flush(mem); /* should call flush ? */ + (void)BIO_flush(mem); /* should call flush ? */ size = BIO_pending(mem); res = xmlMalloc(size + 1); diff --git a/src/xmldsig.c b/src/xmldsig.c index ea4797fe90506fd52cc4fde9c5bc368de1ff4139..a4a322395ce91f0f6861eeebd4aa1856b9c764aa 100644 --- a/src/xmldsig.c +++ b/src/xmldsig.c @@ -1126,9 +1126,9 @@ xmlSecDSigCtxDebugDump(xmlSecDSigCtxPtr dsigCtx, FILE* output) { (xmlSecBufferGetData(dsigCtx->result) != NULL)) { fprintf(output, "== Result - start buffer:\n"); - fwrite(xmlSecBufferGetData(dsigCtx->result), - xmlSecBufferGetSize(dsigCtx->result), - 1, output); + (void)fwrite(xmlSecBufferGetData(dsigCtx->result), + xmlSecBufferGetSize(dsigCtx->result), + 1, output); fprintf(output, "\n== Result - end buffer\n"); } if(((dsigCtx->flags & XMLSEC_DSIG_FLAGS_STORE_SIGNATURE) != 0) && @@ -1136,9 +1136,9 @@ xmlSecDSigCtxDebugDump(xmlSecDSigCtxPtr dsigCtx, FILE* output) { (xmlSecBufferGetData(xmlSecDSigCtxGetPreSignBuffer(dsigCtx)) != NULL)) { fprintf(output, "== PreSigned data - start buffer:\n"); - fwrite(xmlSecBufferGetData(xmlSecDSigCtxGetPreSignBuffer(dsigCtx)), - xmlSecBufferGetSize(xmlSecDSigCtxGetPreSignBuffer(dsigCtx)), - 1, output); + (void)fwrite(xmlSecBufferGetData(xmlSecDSigCtxGetPreSignBuffer(dsigCtx)), + xmlSecBufferGetSize(xmlSecDSigCtxGetPreSignBuffer(dsigCtx)), + 1, output); fprintf(output, "\n== PreSigned data - end buffer\n"); } } @@ -1215,9 +1215,9 @@ xmlSecDSigCtxDebugXmlDump(xmlSecDSigCtxPtr dsigCtx, FILE* output) { (xmlSecBufferGetData(dsigCtx->result) != NULL)) { fprintf(output, "<Result>"); - fwrite(xmlSecBufferGetData(dsigCtx->result), - xmlSecBufferGetSize(dsigCtx->result), - 1, output); + (void)fwrite(xmlSecBufferGetData(dsigCtx->result), + xmlSecBufferGetSize(dsigCtx->result), + 1, output); fprintf(output, "</Result>\n"); } if(((dsigCtx->flags & XMLSEC_DSIG_FLAGS_STORE_SIGNATURE) != 0) && @@ -1225,9 +1225,9 @@ xmlSecDSigCtxDebugXmlDump(xmlSecDSigCtxPtr dsigCtx, FILE* output) { (xmlSecBufferGetData(xmlSecDSigCtxGetPreSignBuffer(dsigCtx)) != NULL)) { fprintf(output, "<PreSignedData>"); - fwrite(xmlSecBufferGetData(xmlSecDSigCtxGetPreSignBuffer(dsigCtx)), - xmlSecBufferGetSize(xmlSecDSigCtxGetPreSignBuffer(dsigCtx)), - 1, output); + (void)fwrite(xmlSecBufferGetData(xmlSecDSigCtxGetPreSignBuffer(dsigCtx)), + xmlSecBufferGetSize(xmlSecDSigCtxGetPreSignBuffer(dsigCtx)), + 1, output); fprintf(output, "</PreSignedData>\n"); } @@ -1677,9 +1677,9 @@ xmlSecDSigReferenceCtxDebugDump(xmlSecDSigReferenceCtxPtr dsigRefCtx, FILE* outp (xmlSecBufferGetData(xmlSecDSigReferenceCtxGetPreDigestBuffer(dsigRefCtx)) != NULL)) { fprintf(output, "== PreDigest data - start buffer:\n"); - fwrite(xmlSecBufferGetData(xmlSecDSigReferenceCtxGetPreDigestBuffer(dsigRefCtx)), - xmlSecBufferGetSize(xmlSecDSigReferenceCtxGetPreDigestBuffer(dsigRefCtx)), - 1, output); + (void)fwrite(xmlSecBufferGetData(xmlSecDSigReferenceCtxGetPreDigestBuffer(dsigRefCtx)), + xmlSecBufferGetSize(xmlSecDSigReferenceCtxGetPreDigestBuffer(dsigRefCtx)), + 1, output); fprintf(output, "\n== PreDigest data - end buffer\n"); } @@ -1687,9 +1687,9 @@ xmlSecDSigReferenceCtxDebugDump(xmlSecDSigReferenceCtxPtr dsigRefCtx, FILE* outp (xmlSecBufferGetData(dsigRefCtx->result) != NULL)) { fprintf(output, "== Result - start buffer:\n"); - fwrite(xmlSecBufferGetData(dsigRefCtx->result), - xmlSecBufferGetSize(dsigRefCtx->result), 1, - output); + (void)fwrite(xmlSecBufferGetData(dsigRefCtx->result), + xmlSecBufferGetSize(dsigRefCtx->result), 1, + output); fprintf(output, "\n== Result - end buffer\n"); } } @@ -1750,9 +1750,9 @@ xmlSecDSigReferenceCtxDebugXmlDump(xmlSecDSigReferenceCtxPtr dsigRefCtx, FILE* o (xmlSecBufferGetData(dsigRefCtx->result) != NULL)) { fprintf(output, "<Result>"); - fwrite(xmlSecBufferGetData(dsigRefCtx->result), - xmlSecBufferGetSize(dsigRefCtx->result), 1, - output); + (void)fwrite(xmlSecBufferGetData(dsigRefCtx->result), + xmlSecBufferGetSize(dsigRefCtx->result), 1, + output); fprintf(output, "</Result>\n"); } @@ -1760,9 +1760,9 @@ xmlSecDSigReferenceCtxDebugXmlDump(xmlSecDSigReferenceCtxPtr dsigRefCtx, FILE* o (xmlSecBufferGetData(xmlSecDSigReferenceCtxGetPreDigestBuffer(dsigRefCtx)) != NULL)) { fprintf(output, "<PreDigestData>"); - fwrite(xmlSecBufferGetData(xmlSecDSigReferenceCtxGetPreDigestBuffer(dsigRefCtx)), - xmlSecBufferGetSize(xmlSecDSigReferenceCtxGetPreDigestBuffer(dsigRefCtx)), - 1, output); + (void)fwrite(xmlSecBufferGetData(xmlSecDSigReferenceCtxGetPreDigestBuffer(dsigRefCtx)), + xmlSecBufferGetSize(xmlSecDSigReferenceCtxGetPreDigestBuffer(dsigRefCtx)), + 1, output); fprintf(output, "</PreDigestData>\n"); } if(dsigRefCtx->dsigCtx->operation == xmlSecTransformOperationSign) { diff --git a/src/xmlenc.c b/src/xmlenc.c index 04801595d093b0ed2aa72cc09a27abf8494b53e1..3d4e0d23f92c6bc14a4c3961fd2a02d49bdbe79d 100644 --- a/src/xmlenc.c +++ b/src/xmlenc.c @@ -1218,9 +1218,9 @@ xmlSecEncCtxDebugDump(xmlSecEncCtxPtr encCtx, FILE* output) { (encCtx->resultBase64Encoded != 0)) { fprintf(output, "== Result - start buffer:\n"); - fwrite(xmlSecBufferGetData(encCtx->result), - xmlSecBufferGetSize(encCtx->result), 1, - output); + (void)fwrite(xmlSecBufferGetData(encCtx->result), + xmlSecBufferGetSize(encCtx->result), 1, + output); fprintf(output, "\n== Result - end buffer\n"); } } @@ -1311,9 +1311,9 @@ xmlSecEncCtxDebugXmlDump(xmlSecEncCtxPtr encCtx, FILE* output) { (encCtx->resultBase64Encoded != 0)) { fprintf(output, "<Result>"); - fwrite(xmlSecBufferGetData(encCtx->result), - xmlSecBufferGetSize(encCtx->result), 1, - output); + (void)fwrite(xmlSecBufferGetData(encCtx->result), + xmlSecBufferGetSize(encCtx->result), 1, + output); fprintf(output, "</Result>\n"); } -- 1.9.3
>From 8b0f844b6830fccc84e13d519d9453634f134f69 Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Tue, 3 Jun 2014 17:02:42 -0400 Subject: [PATCH 6/7] Elimanate assignments that are never used Clang found a number of cases where variables were assigned by neve read. Silence warnings by eliminating useles assignments. Signed-off-by: Simo Sorce <[email protected]> --- src/keysdata.c | 3 +-- src/nss/pkikeys.c | 3 +-- src/nss/x509vfy.c | 2 -- src/openssl/x509vfy.c | 4 +--- src/parser.c | 2 +- 5 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/keysdata.c b/src/keysdata.c index de854ba6cbcb24ab59ede2c116dfab52ee41360b..b2b8d19d06ef35cc5892336b77511feaafd512fa 100644 --- a/src/keysdata.c +++ b/src/keysdata.c @@ -239,7 +239,6 @@ xmlSecKeyDataCreate(xmlSecKeyDataId id) { xmlSecKeyDataPtr xmlSecKeyDataDuplicate(xmlSecKeyDataPtr data) { xmlSecKeyDataPtr newData; - int ret; xmlSecAssert2(xmlSecKeyDataIsValid(data), NULL); xmlSecAssert2(data->id->duplicate != NULL, NULL); @@ -254,7 +253,7 @@ xmlSecKeyDataDuplicate(xmlSecKeyDataPtr data) { return(NULL); } - ret = (data->id->duplicate)(newData, data); + (void)(data->id->duplicate)(newData, data); if(newData == NULL) { xmlSecError(XMLSEC_ERRORS_HERE, xmlSecErrorsSafeString(xmlSecKeyDataGetName(data)), diff --git a/src/nss/pkikeys.c b/src/nss/pkikeys.c index ae9e29b4e82ed5c2a3c0d1db0cd8618ce1c116f7..91885742aab172d064edb38c356830ecf72f7289 100644 --- a/src/nss/pkikeys.c +++ b/src/nss/pkikeys.c @@ -568,7 +568,6 @@ xmlSecNssKeyDataDsaXmlRead(xmlSecKeyDataId id, xmlSecKeyPtr key, xmlNodePtr cur; int ret; PK11SlotInfo *slot = NULL; - CK_OBJECT_HANDLE handle; SECKEYPublicKey *pubkey=NULL; PRArenaPool *arena = NULL; @@ -751,7 +750,7 @@ xmlSecNssKeyDataDsaXmlRead(xmlSecKeyDataId id, xmlSecKeyPtr key, goto done; } - handle = PK11_ImportPublicKey(slot, pubkey, PR_FALSE); + (void)PK11_ImportPublicKey(slot, pubkey, PR_FALSE); data = xmlSecKeyDataCreate(id); if(data == NULL ) { diff --git a/src/nss/x509vfy.c b/src/nss/x509vfy.c index fdb866fe966a5c21e52ca7064c0401e40e6c664f..ae4aca585129ba223a3c32fed7ea57653ff4ee4a 100644 --- a/src/nss/x509vfy.c +++ b/src/nss/x509vfy.c @@ -693,8 +693,6 @@ xmlSecNssX509NameRead(xmlSecByte *str, int len) { if (len > 0) *p++=','; } - } else { - valueLen = 0; } if(len > 0) { ++str; --len; diff --git a/src/openssl/x509vfy.c b/src/openssl/x509vfy.c index ca5a46266c641416991339272f4459c3ac59b557..17771310910ba5785efe777ef36d00584d7724db 100644 --- a/src/openssl/x509vfy.c +++ b/src/openssl/x509vfy.c @@ -178,7 +178,7 @@ xmlSecOpenSSLX509StoreVerify(xmlSecKeyDataStorePtr store, XMLSEC_STACK_OF_X509* X509 * cert; X509 * err_cert = NULL; char buf[256]; - int err = 0, depth; + int err = 0; int i; int ret; @@ -315,7 +315,6 @@ xmlSecOpenSSLX509StoreVerify(xmlSecKeyDataStorePtr store, XMLSEC_STACK_OF_X509* if(keyInfoCtx->certsVerificationTime > 0) { #if !defined(XMLSEC_OPENSSL_096) && !defined(XMLSEC_OPENSSL_097) - vpm_flags |= X509_V_FLAG_USE_CHECK_TIME; X509_VERIFY_PARAM_set_time(vpm, keyInfoCtx->certsVerificationTime); #endif /* !defined(XMLSEC_OPENSSL_096) && !defined(XMLSEC_OPENSSL_097) */ X509_STORE_CTX_set_time(&xsc, 0, keyInfoCtx->certsVerificationTime); @@ -329,7 +328,6 @@ xmlSecOpenSSLX509StoreVerify(xmlSecKeyDataStorePtr store, XMLSEC_STACK_OF_X509* ret = X509_verify_cert(&xsc); err_cert = X509_STORE_CTX_get_current_cert(&xsc); err = X509_STORE_CTX_get_error(&xsc); - depth = X509_STORE_CTX_get_error_depth(&xsc); X509_STORE_CTX_cleanup (&xsc); diff --git a/src/parser.c b/src/parser.c index 990ff98a9c3fbdb3bf6cdd6838c264f8ce0e0010..f289977c91060c69ac9e01287b8f502392125903 100644 --- a/src/parser.c +++ b/src/parser.c @@ -316,7 +316,7 @@ xmlSecParserPopXml(xmlSecTransformPtr transform, xmlSecNodeSetPtr* nodes, return(-1); } - ret = inputPush(ctxt, input); + (void)inputPush(ctxt, input); if(input == NULL) { xmlSecError(XMLSEC_ERRORS_HERE, xmlSecErrorsSafeString(xmlSecTransformGetName(transform)), -- 1.9.3
>From a8d75458a1667b9ebc92bc24f9a8cfef93aa45f9 Mon Sep 17 00:00:00 2001 From: Simo Sorce <[email protected]> Date: Tue, 3 Jun 2014 17:09:40 -0400 Subject: [PATCH 7/7] Fix potential NULL dereference Clang indicates these pointers may be null, so check before dereferencing. Signed-off-by: Simo Sorce <[email protected]> --- src/nss/keysstore.c | 2 +- src/nss/x509vfy.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nss/keysstore.c b/src/nss/keysstore.c index f07e44be2067638a8f22e081b149ca97b78e6e63..057fc454b4efac100030e33a096559397ad8b2d4 100644 --- a/src/nss/keysstore.c +++ b/src/nss/keysstore.c @@ -271,7 +271,7 @@ xmlSecNssKeysStoreInitialize(xmlSecKeyStorePtr store) { xmlSecAssert2(xmlSecKeyStoreCheckId(store, xmlSecNssKeysStoreId), -1); ss = xmlSecNssKeysStoreGetSS(store); - xmlSecAssert2((*ss == NULL), -1); + xmlSecAssert2(((ss == NULL) || (*ss == NULL)), -1); *ss = xmlSecKeyStoreCreate(xmlSecSimpleKeysStoreId); if(*ss == NULL) { diff --git a/src/nss/x509vfy.c b/src/nss/x509vfy.c index ae4aca585129ba223a3c32fed7ea57653ff4ee4a..06e75624bcdee628061032acd0d091fff2b67ca2 100644 --- a/src/nss/x509vfy.c +++ b/src/nss/x509vfy.c @@ -233,7 +233,7 @@ xmlSecNssX509StoreVerify(xmlSecKeyDataStorePtr store, CERTCertList* certs, NULL, XMLSEC_ERRORS_R_CERT_ISSUER_FAILED, "cert with subject name %s could not be verified because the issuer's cert is expired/invalid or not found", - cert->subjectName); + cert?cert->subjectName:"(NULL)"); break; case SEC_ERROR_EXPIRED_CERTIFICATE: xmlSecError(XMLSEC_ERRORS_HERE, @@ -241,7 +241,7 @@ xmlSecNssX509StoreVerify(xmlSecKeyDataStorePtr store, CERTCertList* certs, NULL, XMLSEC_ERRORS_R_CERT_HAS_EXPIRED, "cert with subject name %s has expired", - cert->subjectName); + cert?cert->subjectName:"(NULL)"); break; case SEC_ERROR_REVOKED_CERTIFICATE: xmlSecError(XMLSEC_ERRORS_HERE, @@ -249,7 +249,7 @@ xmlSecNssX509StoreVerify(xmlSecKeyDataStorePtr store, CERTCertList* certs, NULL, XMLSEC_ERRORS_R_CERT_REVOKED, "cert with subject name %s has been revoked", - cert->subjectName); + cert?cert->subjectName:"(NULL)"); break; default: xmlSecError(XMLSEC_ERRORS_HERE, @@ -257,7 +257,7 @@ xmlSecNssX509StoreVerify(xmlSecKeyDataStorePtr store, CERTCertList* certs, NULL, XMLSEC_ERRORS_R_CERT_VERIFY_FAILED, "cert with subject name %s could not be verified, errcode %d", - cert->subjectName, + cert?cert->subjectName:"(NULL)", PORT_GetError()); break; } -- 1.9.3
_______________________________________________ xmlsec mailing list [email protected] http://www.aleksey.com/mailman/listinfo/xmlsec
