From c97803e20287c189e37b5a737e84ed02b510949f Mon Sep 17 00:00:00 2001 Subject: [PATCH] mscng: fix use-after-free, implement adoption of private key as part of key extraction (#192) --- src/mscng/signatures.c | 18 +++++++++++++----- src/mscng/x509.c | 8 ++++++-- src/xmltree.c | 2 +- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/mscng/signatures.c b/src/mscng/signatures.c index 365c484a..a7e0fbb7 100644 --- a/src/mscng/signatures.c +++ b/src/mscng/signatures.c @@ -258,20 +258,28 @@ static void xmlSecMSCngSignatureFinalize(xmlSecTransformPtr transform) { xmlSecKeyDataDestroy(ctx->data); } - if(ctx->pbHash != NULL) { - xmlFree(ctx->pbHash); - } + // MSDN documents at + // https://msdn.microsoft.com/en-us/library/windows/desktop/aa376217(v=vs.85).aspx + // that the order of cleanup should be: + // - algo handle + // - hash handle + // - hash object pointer + // - hash pointer if(ctx->hHashAlg != 0) { BCryptCloseAlgorithmProvider(ctx->hHashAlg, 0); } + if(ctx->hHash != 0) { + BCryptDestroyHash(ctx->hHash); + } + if(ctx->pbHashObject != NULL) { xmlFree(ctx->pbHashObject); } - if(ctx->hHash != 0) { - BCryptDestroyHash(ctx->hHash); + if(ctx->pbHash != NULL) { + xmlFree(ctx->pbHash); } memset(ctx, 0, sizeof(xmlSecMSCngSignatureCtx)); diff --git a/src/mscng/x509.c b/src/mscng/x509.c index 492193af..3ab62c5c 100644 --- a/src/mscng/x509.c +++ b/src/mscng/x509.c @@ -785,8 +785,12 @@ xmlSecMSCngKeyDataX509VerifyAndExtractKey(xmlSecKeyDataPtr data, } if((keyInfoCtx->keyReq.keyType & xmlSecKeyDataTypePrivate) != 0) { - xmlSecNotImplementedError(NULL); - return(-1); + keyValue = xmlSecMSCngCertAdopt(certCopy, xmlSecKeyDataTypePrivate); + if(keyValue == NULL) { + xmlSecInternalError("xmlSecMSCngCertAdopt", + xmlSecKeyDataGetName(data)); + return(-1); + } } else if((keyInfoCtx->keyReq.keyType & xmlSecKeyDataTypePublic) != 0) { keyValue = xmlSecMSCngCertAdopt(certCopy, xmlSecKeyDataTypePublic); if(keyValue == NULL) { -- 2.16.4