From dd68c6dc4b09044811732f2096a7f3454c1988b4 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Sat, 11 Jul 2026 20:52:29 +0200 Subject: [PATCH 1/6] [Android] Add missing JNI exception checks across crypto PAL Adds missing JNI exception checks across the remaining functions in the Android cryptography PAL, based on a static audit of `Call*Method` / `NewObject` / `Get*ArrayRegion`-style JNI calls where a pending Java exception could otherwise leak back into managed code (undefined behavior for the next managed->native transition) or produce partial output. The touched functions follow the patterns already established elsewhere in the PAL: 1. `ON_EXCEPTION_PRINT_AND_GOTO(label)` immediately after each fallible JNI call. The macro both prints and clears the exception, so subsequent JNI calls on the cleanup path are safe. 2. `INIT_LOCALS(loc, ...)` + a single `cleanup:`/`RELEASE_LOCALS(loc, env)` tail for local-ref hygiene, replacing ad-hoc `jobject foo = NULL;` declarations and scattered `DeleteLocalRef` calls. 3. No partial output state on failure -- results are computed into locals and only promoted to caller-visible out-parameters / global refs on the success path. Files changed (all under `src/native/libs/System.Security.Cryptography.Native.Android/` unless noted): - `pal_x509chain.c` -- harden the functions not covered by the earlier `X509ChainCreateContext` fix (build, certificate/error enumeration, custom trust store, revocation validation) and clear exceptions on the error-capture paths. - `pal_bignum.c` -- check `toByteArray()` before using the result. - `pal_cipher.c` -- `CipherSetKeyAndIV`, `CipherUpdateAAD`, `CipherUpdate` (also releases the update output array on all paths). - `pal_ecdsa.c` -- clear the exception on the `getPrivate()` failure path in `EcDsaSign`. - `pal_eckey.c` -- `NewEcKeyFromKeys`, `EcKeyCreateByOid` (converted to `INIT_LOCALS` + per-call checks), `EcKeyGetCurveName`. - `pal_hmac.c` -- check `Mac.doFinal()` before reading the result array. - `pal_misc.c` -- `GetRandomBytes`. - `pal_pbkdf2.c` -- clear the pending exception when `NewDirectByteBuffer` fails. - `pal_sslstream.c` -- `SetTargetHost`, `SSLStreamRead`, `RequestClientAuthentication`, `SetApplicationProtocols`. - `pal_x509.c` -- check `SetByteArrayRegion` in the decode paths. Managed changes: - `Interop.X509Chain.cs` -- `X509ChainGetErrorCount` now returns a negative value when a JNI exception occurs (distinct from a legitimate count of 0); the wrapper throws `CryptographicException` in that case instead of silently reporting "no errors". - `Interop.Bignum.cs` -- `AndroidCryptoNative_BigNumToBinary` now returns -1 when a JNI exception occurs and writes nothing; `ExtractBignum` throws `CryptographicException` rather than returning a zero-filled buffer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc96caa6-f528-4662-8cfa-df04e8a47475 --- .../Interop.Bignum.cs | 8 +- .../Interop.X509Chain.cs | 2 + .../pal_bignum.c | 26 +++- .../pal_cipher.c | 25 +++- .../pal_ecdsa.c | 2 +- .../pal_eckey.c | 88 ++++++------ .../pal_hmac.c | 15 +- .../pal_misc.c | 8 +- .../pal_pbkdf2.c | 5 +- .../pal_sslstream.c | 9 ++ .../pal_x509.c | 3 + .../pal_x509chain.c | 135 ++++++++++++++---- 12 files changed, 239 insertions(+), 87 deletions(-) diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs index 9cf78a9d6b1fae..52f94f6888ddba 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs @@ -35,12 +35,18 @@ internal static partial class Crypto // the array with zeroes. int offset = targetSize - compactSize; byte[] buf = new byte[targetSize]; + int written; fixed (byte* to = buf) { byte* start = to + offset; - BigNumToBinary(bignum, start); + written = BigNumToBinary(bignum, start); } + // A negative result means a pending JNI exception was cleared and no data was written. + // Fail loudly rather than silently returning a zero-filled (corrupt) buffer. + if (written < 0) + throw new CryptographicException(); + return buf; } } diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs index c096cb91350735..89251c82df05c1 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs @@ -112,6 +112,8 @@ private static unsafe partial int X509ChainGetErrors( internal static ValidationError[] X509ChainGetErrors(SafeX509ChainContextHandle ctx) { int count = Interop.AndroidCrypto.X509ChainGetErrorCount(ctx); + if (count < 0) + throw new CryptographicException(); if (count == 0) return Array.Empty(); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c index 67b80859d186b8..b2e7d352aa1689 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c @@ -13,13 +13,18 @@ int32_t AndroidCryptoNative_BigNumToBinary(jobject bignum, uint8_t* output) // bigNum.toByteArray() JNIEnv* env = GetJNIEnv(); + int32_t ret = -1; + jsize bytesLen = 0; + jsize startingIndex = 0; + jbyte leadingByte = 0; + jbyteArray bytes = (jbyteArray)(*env)->CallObjectMethod(env, bignum, g_toByteArrayMethod); - jsize bytesLen = (*env)->GetArrayLength(env, bytes); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + bytesLen = (*env)->GetArrayLength(env, bytes); // We strip the leading zero byte from the byte array. - jsize startingIndex = 0; - jbyte leadingByte; (*env)->GetByteArrayRegion(env, bytes, 0, 1, &leadingByte); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (leadingByte == 0) { startingIndex++; @@ -27,8 +32,12 @@ int32_t AndroidCryptoNative_BigNumToBinary(jobject bignum, uint8_t* output) } (*env)->GetByteArrayRegion(env, bytes, startingIndex, bytesLen, (jbyte*)output); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = (int32_t)bytesLen; + +cleanup: (*env)->DeleteLocalRef(env, bytes); - return CheckJNIExceptions(env) ? FAIL : (int32_t)bytesLen; + return ret; } int32_t AndroidCryptoNative_GetBigNumBytes(jobject bignum) @@ -65,8 +74,13 @@ int32_t AndroidCryptoNative_GetBigNumBytesIncludingPaddingByteForSign(jobject bi // Use the array here to get the leading zero byte if it exists. // bigNum.toByteArray().length(); JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; + jbyteArray bytes = (jbyteArray)(*env)->CallObjectMethod(env, bignum, g_toByteArrayMethod); - jsize bytesLen = (*env)->GetArrayLength(env, bytes); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = (int32_t)(*env)->GetArrayLength(env, bytes); + +cleanup: (*env)->DeleteLocalRef(env, bytes); - return CheckJNIExceptions(env) ? FAIL : (int32_t)bytesLen; + return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c index b140e84b55e552..06b5ec3f23ab9e 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c @@ -208,6 +208,8 @@ int32_t AndroidCryptoNative_CipherSetKeyAndIV(CipherCtx* ctx, uint8_t* key, uint // ivLength = cipher.getBlockSize(); JNIEnv *env = GetJNIEnv(); ctx->ivLength = (*env)->CallIntMethod(env, ctx->cipher, g_getBlockSizeMethod); + if (CheckJNIExceptions(env)) + return FAIL; } SaveTo(iv, &ctx->iv, (size_t)ctx->ivLength, /* overwrite */ true); @@ -241,11 +243,17 @@ int32_t AndroidCryptoNative_CipherUpdateAAD(CipherCtx* ctx, uint8_t* in, int32_t abort_if_invalid_pointer_argument(in); JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; jbyteArray inDataBytes = make_java_byte_array(env, inl); (*env)->SetByteArrayRegion(env, inDataBytes, 0, inl, (jbyte*)in); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, ctx->cipher, g_cipherUpdateAADMethod, inDataBytes); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = SUCCESS; + +cleanup: (*env)->DeleteLocalRef(env, inDataBytes); - return CheckJNIExceptions(env) ? FAIL : SUCCESS; + return ret; } int32_t AndroidCryptoNative_CipherUpdate(CipherCtx* ctx, uint8_t* outm, int32_t* outl, uint8_t* in, int32_t inl) @@ -261,22 +269,29 @@ int32_t AndroidCryptoNative_CipherUpdate(CipherCtx* ctx, uint8_t* outm, int32_t* abort_if_invalid_pointer_argument(in); JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; + jbyteArray outDataBytes = NULL; jbyteArray inDataBytes = make_java_byte_array(env, inl); (*env)->SetByteArrayRegion(env, inDataBytes, 0, inl, (jbyte*)in); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *outl = 0; - jbyteArray outDataBytes = (jbyteArray)(*env)->CallObjectMethod(env, ctx->cipher, g_cipherUpdateMethod, inDataBytes); + outDataBytes = (jbyteArray)(*env)->CallObjectMethod(env, ctx->cipher, g_cipherUpdateMethod, inDataBytes); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (outDataBytes && outm) { jsize outDataBytesLen = (*env)->GetArrayLength(env, outDataBytes); - *outl = outDataBytesLen; (*env)->GetByteArrayRegion(env, outDataBytes, 0, outDataBytesLen, (jbyte*) outm); - (*env)->DeleteLocalRef(env, outDataBytes); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + *outl = outDataBytesLen; } + ret = SUCCESS; +cleanup: + (*env)->DeleteLocalRef(env, outDataBytes); (*env)->DeleteLocalRef(env, inDataBytes); - return CheckJNIExceptions(env) ? FAIL : SUCCESS; + return ret; } int32_t AndroidCryptoNative_CipherFinalEx(CipherCtx* ctx, uint8_t* outm, int32_t* outl) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c index 29de5869702ec7..fb6a603b8cded6 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c @@ -33,7 +33,7 @@ int32_t AndroidCryptoNative_EcDsaSign(const uint8_t* dgst, int32_t dgstlen, uint } jobject privateKey = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPrivateMethod); - if (!privateKey) + if (CheckJNIExceptions(env) || !privateKey) { ReleaseLRef(env, signatureObject); return FAIL; diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c index ccc79a4652a12d..e92478f5e3a69a 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c @@ -26,6 +26,9 @@ EC_KEY* AndroidCryptoNative_NewEcKeyFromKeys(JNIEnv *env, jobject /*ECPublicKey* return NULL; jobject curveParameters = (*env)->CallObjectMethod(env, publicKey, g_ECPublicKeyGetParams); + if (CheckJNIExceptions(env) || curveParameters == NULL) + return NULL; + return AndroidCryptoNative_NewEcKey(ToGRef(env, curveParameters), AndroidCryptoNative_CreateKeyPair(env, publicKey, privateKey)); } @@ -73,73 +76,63 @@ EC_KEY* AndroidCryptoNative_EcKeyCreateByOid(const char* oid) abort_if_invalid_pointer_argument (oid); JNIEnv* env = GetJNIEnv(); + EC_KEY* ret = NULL; + INIT_LOCALS(loc, oidStr, ec, paramSpec, keyPairGenerator, keyPair, keyFactory, publicKey, keySpec, curveParameters); // Older versions of Android don't support mapping an OID to a curve name, // so do some of the common mappings here. - jstring oidStr; if (strcmp(oid, "1.3.132.0.33") == 0) { - oidStr = make_java_string(env, "secp224r1"); + loc[oidStr] = make_java_string(env, "secp224r1"); } else if (strcmp(oid, "1.3.132.0.34") == 0 || strcmp(oid, "nistP384") == 0) { - oidStr = make_java_string(env, "secp384r1"); + loc[oidStr] = make_java_string(env, "secp384r1"); } else if (strcmp(oid, "1.3.132.0.35") == 0 || strcmp(oid, "nistP521") == 0) { - oidStr = make_java_string(env, "secp521r1"); + loc[oidStr] = make_java_string(env, "secp521r1"); } else if (strcmp(oid, "1.2.840.10045.3.1.7") == 0 || strcmp(oid, "nistP256") == 0) { - oidStr = make_java_string(env, "secp256r1"); + loc[oidStr] = make_java_string(env, "secp256r1"); } else { - oidStr = make_java_string(env, oid); + loc[oidStr] = make_java_string(env, oid); } - jstring ec = make_java_string(env, "EC"); + loc[ec] = make_java_string(env, "EC"); // First, generate the key pair based on the curve defined by the oid. - jobject paramSpec = (*env)->NewObject(env, g_ECGenParameterSpecClass, g_ECGenParameterSpecCtor, oidStr); - ReleaseLRef(env, oidStr); - - jobject keyPairGenerator = - (*env)->CallStaticObjectMethod(env, g_keyPairGenClass, g_keyPairGenGetInstanceMethod, ec); - (*env)->CallVoidMethod(env, keyPairGenerator, g_keyPairGenInitializeWithParamsMethod, paramSpec); - - ReleaseLRef(env, paramSpec); - if (CheckJNIExceptions(env)) - { - LOG_DEBUG("Failed to create curve"); - ReleaseLRef(env, ec); - ReleaseLRef(env, keyPairGenerator); - return NULL; - } + loc[paramSpec] = (*env)->NewObject(env, g_ECGenParameterSpecClass, g_ECGenParameterSpecCtor, loc[oidStr]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - jobject keyPair = (*env)->CallObjectMethod(env, keyPairGenerator, g_keyPairGenGenKeyPairMethod); + loc[keyPairGenerator] = + (*env)->CallStaticObjectMethod(env, g_keyPairGenClass, g_keyPairGenGetInstanceMethod, loc[ec]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + (*env)->CallVoidMethod(env, loc[keyPairGenerator], g_keyPairGenInitializeWithParamsMethod, loc[paramSpec]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - ReleaseLRef(env, keyPairGenerator); + loc[keyPair] = (*env)->CallObjectMethod(env, loc[keyPairGenerator], g_keyPairGenGenKeyPairMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // Now that we have the key pair, we can get the curve parameters from the public key. - jobject keyFactory = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, ec); - jobject publicKey = (*env)->CallObjectMethod(env, keyPair, g_keyPairGetPublicMethod); - jobject keySpec = - (*env)->CallObjectMethod(env, keyFactory, g_KeyFactoryGetKeySpecMethod, publicKey, g_ECPublicKeySpecClass); + loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[ec]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + loc[publicKey] = (*env)->CallObjectMethod(env, loc[keyPair], g_keyPairGetPublicMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + loc[keySpec] = + (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[publicKey], g_ECPublicKeySpecClass); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - ReleaseLRef(env, ec); - ReleaseLRef(env, publicKey); - ReleaseLRef(env, keyFactory); + loc[curveParameters] = (*env)->CallObjectMethod(env, loc[keySpec], g_ECPublicKeySpecGetParams); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (CheckJNIExceptions(env)) - { - ReleaseLRef(env, keySpec); - ReleaseLRef(env, keyPair); - return NULL; - } + ret = AndroidCryptoNative_NewEcKey(AddGRef(env, loc[curveParameters]), AddGRef(env, loc[keyPair])); - jobject curveParameters = (*env)->CallObjectMethod(env, keySpec, g_ECPublicKeySpecGetParams); - ReleaseLRef(env, keySpec); - return AndroidCryptoNative_NewEcKey(ToGRef(env, curveParameters), ToGRef(env, keyPair)); +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } int32_t AndroidCryptoNative_EcKeyGetSize(const EC_KEY* key, int32_t* keySize) @@ -187,16 +180,16 @@ int32_t AndroidCryptoNative_EcKeyGetCurveName(const EC_KEY* key, uint16_t** curv jstring curveNameStr = (*env)->CallObjectMethod(env, key->curveParameters, g_ECParameterSpecGetCurveName); - if (!curveNameStr) + if (CheckJNIExceptions(env)) { *curveName = NULL; - return SUCCESS; + return FAIL; } - if (CheckJNIExceptions(env)) + if (!curveNameStr) { *curveName = NULL; - return FAIL; + return SUCCESS; } jsize nameLength = (*env)->GetStringLength(env, curveNameStr); @@ -208,6 +201,13 @@ int32_t AndroidCryptoNative_EcKeyGetCurveName(const EC_KEY* key, uint16_t** curv (*env)->GetStringRegion(env, curveNameStr, 0, nameLength, (jchar*)buffer); (*env)->DeleteLocalRef(env, curveNameStr); + if (CheckJNIExceptions(env)) + { + free(buffer); + *curveName = NULL; + return FAIL; + } + *curveName = buffer; return SUCCESS; diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c index ffced8680d1264..30ad5fd7220d1a 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c @@ -97,14 +97,21 @@ int32_t CryptoNative_HmacUpdate(jobject ctx, uint8_t* data, int32_t len) ARGS_NON_NULL_ALL static int32_t DoFinal(JNIEnv* env, jobject mac, uint8_t* data, int32_t* len) { + int32_t ret = FAIL; + jsize dataBytesLen = 0; + // mac.doFinal(); jbyteArray dataBytes = (jbyteArray)(*env)->CallObjectMethod(env, mac, g_MacDoFinal); - jsize dataBytesLen = (*env)->GetArrayLength(env, dataBytes); - *len = (int32_t)dataBytesLen; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + dataBytesLen = (*env)->GetArrayLength(env, dataBytes); (*env)->GetByteArrayRegion(env, dataBytes, 0, dataBytesLen, (jbyte*) data); - (*env)->DeleteLocalRef(env, dataBytes); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + *len = (int32_t)dataBytesLen; + ret = SUCCESS; - return CheckJNIExceptions(env) ? FAIL : SUCCESS; +cleanup: + (*env)->DeleteLocalRef(env, dataBytes); + return ret; } int32_t CryptoNative_HmacFinal(jobject ctx, uint8_t* data, int32_t* len) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c index 67d3737561e822..a7998139e6c610 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c @@ -15,18 +15,24 @@ int32_t CryptoNative_GetRandomBytes(uint8_t* buff, int32_t len) abort_unless(buff != NULL, "The 'buff' parameter must be a valid pointer"); JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; + jobject randObj = (*env)->NewObject(env, g_randClass, g_randCtor); abort_unless(randObj != NULL,"Unable to create an instance of java/security/SecureRandom"); jbyteArray buffArray = make_java_byte_array(env, len); (*env)->SetByteArrayRegion(env, buffArray, 0, len, (jbyte*)buff); (*env)->CallVoidMethod(env, randObj, g_randNextBytesMethod, buffArray); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->GetByteArrayRegion(env, buffArray, 0, len, (jbyte*)buff); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = SUCCESS; +cleanup: (*env)->DeleteLocalRef(env, buffArray); (*env)->DeleteLocalRef(env, randObj); - return CheckJNIExceptions(env) ? FAIL : SUCCESS; + return ret; } jobject AndroidCryptoNative_CreateKeyPair(JNIEnv* env, jobject publicKey, jobject privateKey) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c index 793b70a74d952c..d4a3bd910a815f 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c @@ -18,8 +18,9 @@ int32_t AndroidCryptoNative_Pbkdf2(const char* algorithmName, jstring javaAlgorithmName = make_java_string(env, algorithmName); jbyteArray passwordBytes = make_java_byte_array(env, passwordLength); - jobject destinationBuffer = (*env)->NewDirectByteBuffer(env, destination, destinationLength); jobject saltByteBuffer = NULL; + jobject destinationBuffer = (*env)->NewDirectByteBuffer(env, destination, destinationLength); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (javaAlgorithmName == NULL || passwordBytes == NULL || destinationBuffer == NULL) { @@ -29,11 +30,13 @@ int32_t AndroidCryptoNative_Pbkdf2(const char* algorithmName, if (password && passwordLength > 0) { (*env)->SetByteArrayRegion(env, passwordBytes, 0, passwordLength, (const jbyte*)password); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } if (salt && saltLength > 0) { saltByteBuffer = (*env)->NewDirectByteBuffer(env, salt, saltLength); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (saltByteBuffer == NULL) { diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c index 9c3a1eb06d6b9e..1672b0145f4144 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c @@ -786,7 +786,9 @@ int32_t AndroidCryptoNative_SSLStreamSetTargetHost(SSLStream* sslStream, const c loc[params] = (*env)->CallObjectMethod(env, sslStream->sslEngine, g_SSLEngineGetSSLParameters); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, loc[params], g_SSLParametersSetServerNames, loc[nameList]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineSetSSLParameters, loc[params]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = SUCCESS; @@ -844,7 +846,9 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t */ IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferFlip)); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); int32_t rem = (*env)->CallIntMethod(env, sslStream->appInBuffer, g_ByteBufferRemaining); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (rem == 0) { IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferCompact)); @@ -859,6 +863,7 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t } IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferFlip)); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (IsHandshaking(handshakeStatus)) { @@ -867,6 +872,7 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t } rem = (*env)->CallIntMethod(env, sslStream->appInBuffer, g_ByteBufferRemaining); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } if (rem > 0) @@ -1122,6 +1128,8 @@ void AndroidCryptoNative_SSLStreamRequestClientAuthentication(SSLStream* sslStre // sslEngine.setWantClientAuth(true); (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineSetWantClientAuth, true); + // This function cannot report failure, so at least ensure a thrown exception doesn't leak to the next JNI call. + (void)CheckJNIExceptions(env); } int32_t AndroidCryptoNative_SSLStreamSetApplicationProtocols(SSLStream* sslStream, @@ -1165,6 +1173,7 @@ int32_t AndroidCryptoNative_SSLStreamSetApplicationProtocols(SSLStream* sslStrea (*env)->CallVoidMethod(env, loc[params], g_SSLParametersSetApplicationProtocols, loc[protocols]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineSetSSLParameters, loc[params]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = SUCCESS; diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509.c index f9c0b50505179a..435f2c1b7ad33f 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509.c @@ -34,6 +34,7 @@ jobject /*X509Certificate*/ AndroidCryptoNative_X509Decode(const uint8_t* buf, i loc[bytes] = make_java_byte_array(env, len); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->SetByteArrayRegion(env, loc[bytes], 0, len, (const jbyte*)buf); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[stream] = (*env)->NewObject(env, g_ByteArrayInputStreamClass, g_ByteArrayInputStreamCtor, loc[bytes]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); @@ -88,6 +89,7 @@ int32_t AndroidCryptoNative_X509DecodeCollection(const uint8_t* buf, // InputStream stream = new ByteArrayInputStream(bytes); loc[bytes] = make_java_byte_array(env, bufLen); (*env)->SetByteArrayRegion(env, loc[bytes], 0, bufLen, (const jbyte*)buf); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[stream] = (*env)->NewObject(env, g_ByteArrayInputStreamClass, g_ByteArrayInputStreamCtor, loc[bytes]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); @@ -215,6 +217,7 @@ PAL_X509ContentType AndroidCryptoNative_X509GetContentType(const uint8_t* buf, i // InputStream stream = new ByteArrayInputStream(bytes); loc[bytes] = make_java_byte_array(env, len); (*env)->SetByteArrayRegion(env, loc[bytes], 0, len, (const jbyte*)buf); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[stream] = (*env)->NewObject(env, g_ByteArrayInputStreamClass, g_ByteArrayInputStreamCtor, loc[bytes]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c index 7fb44cf517ac9a..7a8dacf60658c7 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c @@ -131,12 +131,15 @@ int32_t AndroidCryptoNative_X509ChainBuild(X509ChainContext* ctx, int64_t timeIn // Date date = new Date(timeInMsFromUnixEpoch); // params.setDate(date); loc[date] = (*env)->NewObject(env, g_DateClass, g_DateCtor, timeInMsFromUnixEpoch); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetDate, loc[date]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // Disable revocation checking when building the cert path. It will be handled in a validation pass if the path is // successfully built. // params.setRevocationEnabled(false); (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, false); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // String builderType = "PKIX"; // CertPathBuilder builder = CertPathBuilder.getInstance(builderType); @@ -144,15 +147,20 @@ int32_t AndroidCryptoNative_X509ChainBuild(X509ChainContext* ctx, int64_t timeIn loc[builderType] = make_java_string(env, "PKIX"); loc[builder] = (*env)->CallStaticObjectMethod(env, g_CertPathBuilderClass, g_CertPathBuilderGetInstance, loc[builderType]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[result] = (*env)->CallObjectMethod(env, loc[builder], g_CertPathBuilderBuild, params); if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + // Clear any exception raised while recording the build failure so it doesn't leak to managed code. + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); goto cleanup; } loc[certPath] = (*env)->CallObjectMethod(env, loc[result], g_PKIXCertPathBuilderResultGetCertPath); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[trustAnchor] = (*env)->CallObjectMethod(env, loc[result], g_PKIXCertPathBuilderResultGetTrustAnchor); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ctx->certPath = AddGRef(env, loc[certPath]); ctx->trustAnchor = AddGRef(env, loc[trustAnchor]); @@ -168,12 +176,20 @@ int32_t AndroidCryptoNative_X509ChainGetCertificateCount(X509ChainContext* ctx) abort_if_invalid_pointer_argument (ctx); JNIEnv* env = GetJNIEnv(); + int32_t ret = 0; + int certCount = 0; + // List certPathList = certPath.getCertificates(); jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); - int certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + ret = certCount + 1; // +1 for the trust anchor +cleanup: (*env)->DeleteLocalRef(env, certPathList); - return certCount + 1; // +1 for the trust anchor + return ret; } int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, @@ -184,10 +200,15 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + int certCount = 0; + int32_t i = 0; + jobject trustedCert = NULL; // List certPathList = certPath.getCertificates(); jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); - int certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (certsLen < certCount + 1) goto cleanup; @@ -197,7 +218,6 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, // Certificate cert = certPathList.get(i); // certs[i] = cert; // } - int32_t i; for (i = 0; i < certCount; ++i) { jobject cert = (*env)->CallObjectMethod(env, certPathList, g_ListGet, i); @@ -207,7 +227,8 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, // Certificate trustedCert = trustAnchor.getTrustedCert(); // certs[i] = trustedCert; - jobject trustedCert = (*env)->CallObjectMethod(env, ctx->trustAnchor, g_TrustAnchorGetTrustedCert); + trustedCert = (*env)->CallObjectMethod(env, ctx->trustAnchor, g_TrustAnchorGetTrustedCert); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (i == 0 || !(*env)->IsSameObject(env, certs[i-1], trustedCert)) { certs[i] = ToGRef(env, trustedCert); @@ -230,12 +251,21 @@ int32_t AndroidCryptoNative_X509ChainGetErrorCount(X509ChainContext* ctx) abort_unless(ctx->errorList != NULL, "errorList is NULL in X509ChainContext"); JNIEnv* env = GetJNIEnv(); - int32_t count = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); + int32_t count = -1; + int32_t errorCount = 0; + int32_t revocationErrorCount = 0; + + errorCount = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (ctx->revocationErrorList != NULL) { - count += (*env)->CallIntMethod(env, ctx->revocationErrorList, g_CollectionSize); + revocationErrorCount = (*env)->CallIntMethod(env, ctx->revocationErrorList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } + count = errorCount + revocationErrorCount; + +cleanup: return count; } @@ -310,34 +340,41 @@ static PAL_X509ChainStatusFlags ChainStatusFromValidatorExceptionReason(JNIEnv* return PAL_X509ChainPartialChain; } -static void PopulateValidationError(JNIEnv* env, jobject error, bool isRevocationError, ValidationError* out) +static int32_t PopulateValidationError(JNIEnv* env, jobject error, bool isRevocationError, ValidationError* out) { + int32_t ret = FAIL; int index = -1; PAL_X509ChainStatusFlags chainStatus = PAL_X509ChainNoError; + uint16_t* messagePtr = NULL; + jobject reason = NULL; + jobject message = NULL; + if ((*env)->IsInstanceOf(env, error, g_CertPathValidatorExceptionClass)) { index = (*env)->CallIntMethod(env, error, g_CertPathValidatorExceptionGetIndex); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // Get the reason and convert it to a chain status flag. - jobject reason = (*env)->CallObjectMethod(env, error, g_CertPathValidatorExceptionGetReason); + reason = (*env)->CallObjectMethod(env, error, g_CertPathValidatorExceptionGetReason); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); chainStatus = ChainStatusFromValidatorExceptionReason(env, reason); - (*env)->DeleteLocalRef(env, reason); } else { chainStatus = isRevocationError ? PAL_X509ChainRevocationStatusUnknown : PAL_X509ChainPartialChain; } - jobject message = (*env)->CallObjectMethod(env, error, g_ThrowableGetMessage); - uint16_t* messagePtr = NULL; + message = (*env)->CallObjectMethod(env, error, g_ThrowableGetMessage); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (message != NULL) { - jsize messageLen = message == NULL ? 0 : (*env)->GetStringLength(env, message); + jsize messageLen = (*env)->GetStringLength(env, message); // +1 for null terminator messagePtr = xmalloc(sizeof(uint16_t) * (size_t)(messageLen + 1)); messagePtr[messageLen] = '\0'; (*env)->GetStringRegion(env, message, 0, messageLen, (jchar*)messagePtr); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } // If the error is known to be from revocation checking, but couldn't be mapped to a revocation status, @@ -350,8 +387,14 @@ static void PopulateValidationError(JNIEnv* env, jobject error, bool isRevocatio out->message = messagePtr; out->index = index; out->chainStatus = chainStatus; + messagePtr = NULL; + ret = SUCCESS; +cleanup: + free(messagePtr); + (*env)->DeleteLocalRef(env, reason); (*env)->DeleteLocalRef(env, message); + return ret; } int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, ValidationError* errors, int32_t errorsLen) @@ -362,10 +405,15 @@ int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, Validation JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + int32_t populated = 0; + int32_t errorCount = 0; + int32_t revocationErrorCount = 0; - int32_t errorCount = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); - int32_t revocationErrorCount = + errorCount = (*env)->CallIntMethod(env, ctx->errorList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(exit); + revocationErrorCount = ctx->revocationErrorList == NULL ? 0 : (*env)->CallIntMethod(env, ctx->revocationErrorList, g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(exit); if (errorsLen < errorCount + revocationErrorCount) goto exit; @@ -380,8 +428,11 @@ int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, Validation { jobject error = (*env)->CallObjectMethod(env, ctx->errorList, g_ListGet, i); ON_EXCEPTION_PRINT_AND_GOTO(exit); - PopulateValidationError(env, error, false /*isRevocationError*/, &errors[i]); + int32_t status = PopulateValidationError(env, error, false /*isRevocationError*/, &errors[populated]); (*env)->DeleteLocalRef(env, error); + if (status != SUCCESS) + goto exit; + populated++; } // for (int i = 0; i < revocationErrorList.size(); ++i) { @@ -393,14 +444,26 @@ int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, Validation { jobject error = (*env)->CallObjectMethod(env, ctx->revocationErrorList, g_ListGet, i); ON_EXCEPTION_PRINT_AND_GOTO(exit); - PopulateValidationError(env, error, true /*isRevocationError*/, &errors[errorCount + i]); + int32_t status = PopulateValidationError(env, error, true /*isRevocationError*/, &errors[populated]); (*env)->DeleteLocalRef(env, error); + if (status != SUCCESS) + goto exit; + populated++; } } ret = SUCCESS; exit: + if (ret != SUCCESS) + { + // The managed caller only frees ValidationError.message on the success path, so release any + // buffers already populated here to avoid leaking them when we fail partway through. + for (int32_t i = 0; i < populated; ++i) + { + free(errors[i].message); + } + } return ret; } @@ -414,24 +477,32 @@ int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainContext* ctx, } JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; + // HashSet anchors = new HashSet(customTrustStoreLen); // for (Certificate cert : customTrustStore) { // TrustAnchor anchor = new TrustAnchor(cert, null); // anchors.Add(anchor); // } jobject anchors = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, customTrustStoreLen); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); for (int i = 0; i < customTrustStoreLen; ++i) { jobject anchor = (*env)->NewObject(env, g_TrustAnchorClass, g_TrustAnchorCtor, customTrustStore[i], NULL); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallBooleanMethod(env, anchors, g_HashSetAdd, anchor); (*env)->DeleteLocalRef(env, anchor); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } // params.setTrustAnchors(anchors); (*env)->CallVoidMethod(env, ctx->params, g_PKIXBuilderParametersSetTrustAnchors, anchors); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = SUCCESS; +cleanup: (*env)->DeleteLocalRef(env, anchors); - return CheckJNIExceptions(env) ? FAIL : SUCCESS; + return ret; } static jobject /*CertPath*/ CreateCertPathFromAnchor(JNIEnv* env, jobject /*TrustAnchor*/ trustAnchor) @@ -441,11 +512,14 @@ static jobject /*CertPath*/ CreateCertPathFromAnchor(JNIEnv* env, jobject /*Trus // ArrayList certList = new ArrayList(1); loc[certList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtorWithCapacity, 1); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // Certificate trustedCert = trustAnchor.getTrustedCert(); // certList.add(trustedCert); loc[trustedCert] = (*env)->CallObjectMethod(env, trustAnchor, g_TrustAnchorGetTrustedCert); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, loc[trustedCert]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // CertificateFactory certFactory = CertificateFactory.getInstance("X.509"); loc[certFactoryType] = make_java_string(env, "X.509"); @@ -475,7 +549,7 @@ static int32_t ValidateWithRevocation(JNIEnv* env, abort_if_invalid_pointer_argument (validator); int32_t ret = FAIL; - INIT_LOCALS(loc, certPathFromAnchor, options, checker, result, ex); + INIT_LOCALS(loc, certPathList, certPathFromAnchor, options, checker, result, ex, revocationErrorList); if (revocationMode == X509RevocationMode_Offline) { @@ -495,14 +569,18 @@ static int32_t ValidateWithRevocation(JNIEnv* env, { // List certPathList = certPath.getCertificates(); // int certCount = certPathList.size(); - jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); - int certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + loc[certPathList] = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + int certCount = (int)(*env)->CallIntMethod(env, loc[certPathList], g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (certCount == 0) { // If the chain only consists only of the trust anchor, create a path with just // the trust anchor for revocation checking for end certificate only. This should // still pass normal (non-revocation) validation when it is the only certificate. loc[certPathFromAnchor] = CreateCertPathFromAnchor(env, ctx->trustAnchor); + if (loc[certPathFromAnchor] == NULL) + goto cleanup; certPathToUse = loc[certPathFromAnchor]; } else @@ -515,13 +593,13 @@ static int32_t ValidateWithRevocation(JNIEnv* env, // HashSet options = new HashSet(3); // options.add(PKIXRevocationChecker.Option.ONLY_END_ENTITY); loc[options] = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, 3); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); jobject endOnly = (*env)->GetStaticObjectField( env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionOnlyEndEntity); (*env)->CallBooleanMethod(env, loc[options], g_HashSetAdd, endOnly); (*env)->DeleteLocalRef(env, endOnly); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } - - (*env)->DeleteLocalRef(env, certPathList); } else { @@ -540,23 +618,30 @@ static int32_t ValidateWithRevocation(JNIEnv* env, { // checker.setOptions(options); (*env)->CallVoidMethod(env, loc[checker], g_PKIXRevocationCheckerSetOptions, loc[options]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } // params.addCertPathChecker(checker); (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersAddCertPathChecker, loc[checker]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // params.setRevocationEnabled(true); // PKIXCertPathValidatorResult result = (PKIXCertPathValidatorResult)validator.validate(certPathToUse, params); (*env)->CallVoidMethod(env, params, g_PKIXBuilderParametersSetRevocationEnabled, true); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[result] = (*env)->CallObjectMethod(env, validator, g_CertPathValidatorValidate, certPathToUse, params); if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { if (ctx->revocationErrorList == NULL) { - ctx->revocationErrorList = ToGRef(env, (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor)); + loc[revocationErrorList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtor); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ctx->revocationErrorList = AddGRef(env, loc[revocationErrorList]); } (*env)->CallBooleanMethod(env, ctx->revocationErrorList, g_ArrayListAdd, loc[ex]); + // Clear any exception raised while recording the revocation failure so it doesn't leak to managed code. + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } ret = SUCCESS; @@ -592,6 +677,8 @@ int32_t AndroidCryptoNative_X509ChainValidate(X509ChainContext* ctx, if (TryGetJNIException(env, &loc[ex], false /*printException*/)) { (*env)->CallBooleanMethod(env, ctx->errorList, g_ArrayListAdd, loc[ex]); + // Clear any exception raised while recording the validation failure so it doesn't leak to managed code. + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = SUCCESS; } else if (revocationMode != X509RevocationMode_NoCheck) From ba0fa4d281b3844ffac5c25c32b17adcbb99cb38 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Sat, 11 Jul 2026 23:53:20 +0200 Subject: [PATCH 2/6] [Android] Use INIT_LOCALS + RELEASE_LOCALS and guard out-params in touched crypto PAL functions Refinement pass over the functions touched earlier in this PR: - Convert manual `jobject foo = ...;` + `DeleteLocalRef` local-ref handling to `INIT_LOCALS(loc, ...)` + a single `cleanup:`/`RELEASE_LOCALS(loc, env)` tail in the touched functions (pal_bignum.c, pal_cipher.c, pal_ecdsa.c, pal_eckey.c, pal_hmac.c, pal_misc.c, pal_pbkdf2.c, pal_x509chain.c). - In `AndroidCryptoNative_X509ChainGetCertificates`, the trust-anchor local ref now lives in the `INIT_LOCALS` array and the caller-owned global ref is produced with `AddGRef` (instead of `ToGRef`); `RELEASE_LOCALS` frees the local ref on all paths, which also fixes a local-ref leak on the branch where the trust anchor equalled the last path certificate. - Ensure caller-visible out-parameters are zero-initialized at function entry and only assigned their meaningful value on the success path (`CipherUpdate`, `DoFinal`, `EcKeyGetCurveName`). No functional change on the success paths. Native build (android-arm64 Release) is clean with 0 warnings / 0 errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc96caa6-f528-4662-8cfa-df04e8a47475 --- .../pal_bignum.c | 18 ++++--- .../pal_cipher.c | 30 +++++------ .../pal_ecdsa.c | 28 +++++------ .../pal_eckey.c | 44 ++++++++-------- .../pal_hmac.c | 11 ++-- .../pal_misc.c | 16 +++--- .../pal_pbkdf2.c | 23 ++++----- .../pal_x509chain.c | 50 +++++++++---------- 8 files changed, 111 insertions(+), 109 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c index b2e7d352aa1689..5d0af2e17d23cc 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_bignum.c @@ -17,13 +17,14 @@ int32_t AndroidCryptoNative_BigNumToBinary(jobject bignum, uint8_t* output) jsize bytesLen = 0; jsize startingIndex = 0; jbyte leadingByte = 0; + INIT_LOCALS(loc, bytes); - jbyteArray bytes = (jbyteArray)(*env)->CallObjectMethod(env, bignum, g_toByteArrayMethod); + loc[bytes] = (jbyteArray)(*env)->CallObjectMethod(env, bignum, g_toByteArrayMethod); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - bytesLen = (*env)->GetArrayLength(env, bytes); + bytesLen = (*env)->GetArrayLength(env, loc[bytes]); // We strip the leading zero byte from the byte array. - (*env)->GetByteArrayRegion(env, bytes, 0, 1, &leadingByte); + (*env)->GetByteArrayRegion(env, loc[bytes], 0, 1, &leadingByte); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (leadingByte == 0) { @@ -31,12 +32,12 @@ int32_t AndroidCryptoNative_BigNumToBinary(jobject bignum, uint8_t* output) bytesLen--; } - (*env)->GetByteArrayRegion(env, bytes, startingIndex, bytesLen, (jbyte*)output); + (*env)->GetByteArrayRegion(env, loc[bytes], startingIndex, bytesLen, (jbyte*)output); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = (int32_t)bytesLen; cleanup: - (*env)->DeleteLocalRef(env, bytes); + RELEASE_LOCALS(loc, env); return ret; } @@ -75,12 +76,13 @@ int32_t AndroidCryptoNative_GetBigNumBytesIncludingPaddingByteForSign(jobject bi // bigNum.toByteArray().length(); JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + INIT_LOCALS(loc, bytes); - jbyteArray bytes = (jbyteArray)(*env)->CallObjectMethod(env, bignum, g_toByteArrayMethod); + loc[bytes] = (jbyteArray)(*env)->CallObjectMethod(env, bignum, g_toByteArrayMethod); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - ret = (int32_t)(*env)->GetArrayLength(env, bytes); + ret = (int32_t)(*env)->GetArrayLength(env, loc[bytes]); cleanup: - (*env)->DeleteLocalRef(env, bytes); + RELEASE_LOCALS(loc, env); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c index 06b5ec3f23ab9e..3c3b19e94e5d5b 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_cipher.c @@ -244,15 +244,16 @@ int32_t AndroidCryptoNative_CipherUpdateAAD(CipherCtx* ctx, uint8_t* in, int32_t JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - jbyteArray inDataBytes = make_java_byte_array(env, inl); - (*env)->SetByteArrayRegion(env, inDataBytes, 0, inl, (jbyte*)in); + INIT_LOCALS(loc, inDataBytes); + loc[inDataBytes] = make_java_byte_array(env, inl); + (*env)->SetByteArrayRegion(env, loc[inDataBytes], 0, inl, (jbyte*)in); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - (*env)->CallVoidMethod(env, ctx->cipher, g_cipherUpdateAADMethod, inDataBytes); + (*env)->CallVoidMethod(env, ctx->cipher, g_cipherUpdateAADMethod, loc[inDataBytes]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = SUCCESS; cleanup: - (*env)->DeleteLocalRef(env, inDataBytes); + RELEASE_LOCALS(loc, env); return ret; } @@ -270,27 +271,28 @@ int32_t AndroidCryptoNative_CipherUpdate(CipherCtx* ctx, uint8_t* outm, int32_t* JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - jbyteArray outDataBytes = NULL; - jbyteArray inDataBytes = make_java_byte_array(env, inl); - (*env)->SetByteArrayRegion(env, inDataBytes, 0, inl, (jbyte*)in); - ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + INIT_LOCALS(loc, inDataBytes, outDataBytes); *outl = 0; - outDataBytes = (jbyteArray)(*env)->CallObjectMethod(env, ctx->cipher, g_cipherUpdateMethod, inDataBytes); + + loc[inDataBytes] = make_java_byte_array(env, inl); + (*env)->SetByteArrayRegion(env, loc[inDataBytes], 0, inl, (jbyte*)in); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (outDataBytes && outm) + loc[outDataBytes] = (jbyteArray)(*env)->CallObjectMethod(env, ctx->cipher, g_cipherUpdateMethod, loc[inDataBytes]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + if (loc[outDataBytes] && outm) { - jsize outDataBytesLen = (*env)->GetArrayLength(env, outDataBytes); - (*env)->GetByteArrayRegion(env, outDataBytes, 0, outDataBytesLen, (jbyte*) outm); + jsize outDataBytesLen = (*env)->GetArrayLength(env, loc[outDataBytes]); + (*env)->GetByteArrayRegion(env, loc[outDataBytes], 0, outDataBytesLen, (jbyte*) outm); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *outl = outDataBytesLen; } ret = SUCCESS; cleanup: - (*env)->DeleteLocalRef(env, outDataBytes); - (*env)->DeleteLocalRef(env, inDataBytes); + RELEASE_LOCALS(loc, env); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c index fb6a603b8cded6..ab2909ce8f887e 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdsa.c @@ -25,24 +25,22 @@ int32_t AndroidCryptoNative_EcDsaSign(const uint8_t* dgst, int32_t dgstlen, uint abort_if_invalid_pointer_argument (siglen); JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; + INIT_LOCALS(loc, signatureObject, privateKey); - jobject signatureObject = GetEcDsaSignatureObject(env); - if (!signatureObject) - { - return FAIL; - } + loc[signatureObject] = GetEcDsaSignatureObject(env); + if (!loc[signatureObject]) + goto cleanup; - jobject privateKey = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPrivateMethod); - if (CheckJNIExceptions(env) || !privateKey) - { - ReleaseLRef(env, signatureObject); - return FAIL; - } + loc[privateKey] = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPrivateMethod); + if (CheckJNIExceptions(env) || !loc[privateKey]) + goto cleanup; - int32_t returnValue = AndroidCryptoNative_SignWithSignatureObject(env, signatureObject, privateKey, dgst, dgstlen, sig, siglen); - ReleaseLRef(env, privateKey); - ReleaseLRef(env, signatureObject); - return returnValue; + ret = AndroidCryptoNative_SignWithSignatureObject(env, loc[signatureObject], loc[privateKey], dgst, dgstlen, sig, siglen); + +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } int32_t AndroidCryptoNative_EcDsaVerify(const uint8_t* dgst, int32_t dgstlen, const uint8_t* sig, int32_t siglen, EC_KEY* key) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c index e92478f5e3a69a..75b296d28cb4f1 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c @@ -168,47 +168,47 @@ int32_t AndroidCryptoNative_EcKeyGetSize(const EC_KEY* key, int32_t* keySize) int32_t AndroidCryptoNative_EcKeyGetCurveName(const EC_KEY* key, uint16_t** curveName) { abort_if_invalid_pointer_argument (curveName); + + *curveName = NULL; + if (!g_ECParameterSpecGetCurveName) { // We can't get the curve name. Treat all curves as unnamed. - *curveName = NULL; return SUCCESS; } abort_if_invalid_pointer_argument (key); JNIEnv* env = GetJNIEnv(); - jstring curveNameStr = (*env)->CallObjectMethod(env, key->curveParameters, g_ECParameterSpecGetCurveName); + int32_t ret = FAIL; + uint16_t* buffer = NULL; + INIT_LOCALS(loc, curveNameStr); - if (CheckJNIExceptions(env)) - { - *curveName = NULL; - return FAIL; - } + loc[curveNameStr] = (*env)->CallObjectMethod(env, key->curveParameters, g_ECParameterSpecGetCurveName); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (!curveNameStr) + if (!loc[curveNameStr]) { - *curveName = NULL; - return SUCCESS; + // No curve name available; treat as an unnamed curve. + ret = SUCCESS; + goto cleanup; } - jsize nameLength = (*env)->GetStringLength(env, curveNameStr); + jsize nameLength = (*env)->GetStringLength(env, loc[curveNameStr]); // add one for the null terminator. - uint16_t* buffer = xmalloc(sizeof(int16_t) * (size_t)(nameLength + 1)); + buffer = xmalloc(sizeof(int16_t) * (size_t)(nameLength + 1)); buffer[nameLength] = 0; - (*env)->GetStringRegion(env, curveNameStr, 0, nameLength, (jchar*)buffer); - (*env)->DeleteLocalRef(env, curveNameStr); - - if (CheckJNIExceptions(env)) - { - free(buffer); - *curveName = NULL; - return FAIL; - } + (*env)->GetStringRegion(env, loc[curveNameStr], 0, nameLength, (jchar*)buffer); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *curveName = buffer; + buffer = NULL; + ret = SUCCESS; - return SUCCESS; +cleanup: + free(buffer); + RELEASE_LOCALS(loc, env); + return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c index 30ad5fd7220d1a..69bdaa2c189538 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c @@ -99,18 +99,21 @@ ARGS_NON_NULL_ALL static int32_t DoFinal(JNIEnv* env, jobject mac, uint8_t* data { int32_t ret = FAIL; jsize dataBytesLen = 0; + INIT_LOCALS(loc, dataBytes); + + *len = 0; // mac.doFinal(); - jbyteArray dataBytes = (jbyteArray)(*env)->CallObjectMethod(env, mac, g_MacDoFinal); + loc[dataBytes] = (jbyteArray)(*env)->CallObjectMethod(env, mac, g_MacDoFinal); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - dataBytesLen = (*env)->GetArrayLength(env, dataBytes); - (*env)->GetByteArrayRegion(env, dataBytes, 0, dataBytesLen, (jbyte*) data); + dataBytesLen = (*env)->GetArrayLength(env, loc[dataBytes]); + (*env)->GetByteArrayRegion(env, loc[dataBytes], 0, dataBytesLen, (jbyte*) data); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *len = (int32_t)dataBytesLen; ret = SUCCESS; cleanup: - (*env)->DeleteLocalRef(env, dataBytes); + RELEASE_LOCALS(loc, env); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c index a7998139e6c610..fb08ce5619a5ff 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c @@ -16,21 +16,21 @@ int32_t CryptoNative_GetRandomBytes(uint8_t* buff, int32_t len) JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + INIT_LOCALS(loc, randObj, buffArray); - jobject randObj = (*env)->NewObject(env, g_randClass, g_randCtor); - abort_unless(randObj != NULL,"Unable to create an instance of java/security/SecureRandom"); + loc[randObj] = (*env)->NewObject(env, g_randClass, g_randCtor); + abort_unless(loc[randObj] != NULL,"Unable to create an instance of java/security/SecureRandom"); - jbyteArray buffArray = make_java_byte_array(env, len); - (*env)->SetByteArrayRegion(env, buffArray, 0, len, (jbyte*)buff); - (*env)->CallVoidMethod(env, randObj, g_randNextBytesMethod, buffArray); + loc[buffArray] = make_java_byte_array(env, len); + (*env)->SetByteArrayRegion(env, loc[buffArray], 0, len, (jbyte*)buff); + (*env)->CallVoidMethod(env, loc[randObj], g_randNextBytesMethod, loc[buffArray]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - (*env)->GetByteArrayRegion(env, buffArray, 0, len, (jbyte*)buff); + (*env)->GetByteArrayRegion(env, loc[buffArray], 0, len, (jbyte*)buff); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = SUCCESS; cleanup: - (*env)->DeleteLocalRef(env, buffArray); - (*env)->DeleteLocalRef(env, randObj); + RELEASE_LOCALS(loc, env); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c index d4a3bd910a815f..9cede1619c3b88 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_pbkdf2.c @@ -15,37 +15,37 @@ int32_t AndroidCryptoNative_Pbkdf2(const char* algorithmName, { JNIEnv* env = GetJNIEnv(); jint ret = FAIL; + INIT_LOCALS(loc, javaAlgorithmName, passwordBytes, saltByteBuffer, destinationBuffer); - jstring javaAlgorithmName = make_java_string(env, algorithmName); - jbyteArray passwordBytes = make_java_byte_array(env, passwordLength); - jobject saltByteBuffer = NULL; - jobject destinationBuffer = (*env)->NewDirectByteBuffer(env, destination, destinationLength); + loc[javaAlgorithmName] = make_java_string(env, algorithmName); + loc[passwordBytes] = make_java_byte_array(env, passwordLength); + loc[destinationBuffer] = (*env)->NewDirectByteBuffer(env, destination, destinationLength); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (javaAlgorithmName == NULL || passwordBytes == NULL || destinationBuffer == NULL) + if (loc[javaAlgorithmName] == NULL || loc[passwordBytes] == NULL || loc[destinationBuffer] == NULL) { goto cleanup; } if (password && passwordLength > 0) { - (*env)->SetByteArrayRegion(env, passwordBytes, 0, passwordLength, (const jbyte*)password); + (*env)->SetByteArrayRegion(env, loc[passwordBytes], 0, passwordLength, (const jbyte*)password); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } if (salt && saltLength > 0) { - saltByteBuffer = (*env)->NewDirectByteBuffer(env, salt, saltLength); + loc[saltByteBuffer] = (*env)->NewDirectByteBuffer(env, salt, saltLength); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (saltByteBuffer == NULL) + if (loc[saltByteBuffer] == NULL) { goto cleanup; } } ret = (*env)->CallStaticIntMethod(env, g_PalPbkdf2, g_PalPbkdf2Pbkdf2OneShot, - javaAlgorithmName, passwordBytes, saltByteBuffer, iterations, destinationBuffer); + loc[javaAlgorithmName], loc[passwordBytes], loc[saltByteBuffer], iterations, loc[destinationBuffer]); if (CheckJNIExceptions(env)) { @@ -53,10 +53,7 @@ int32_t AndroidCryptoNative_Pbkdf2(const char* algorithmName, } cleanup: - (*env)->DeleteLocalRef(env, javaAlgorithmName); - (*env)->DeleteLocalRef(env, passwordBytes); - (*env)->DeleteLocalRef(env, saltByteBuffer); - (*env)->DeleteLocalRef(env, destinationBuffer); + RELEASE_LOCALS(loc, env); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c index 7a8dacf60658c7..358923988e19a0 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c @@ -178,17 +178,18 @@ int32_t AndroidCryptoNative_X509ChainGetCertificateCount(X509ChainContext* ctx) int32_t ret = 0; int certCount = 0; + INIT_LOCALS(loc, certPathList); // List certPathList = certPath.getCertificates(); - jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); + loc[certPathList] = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + certCount = (int)(*env)->CallIntMethod(env, loc[certPathList], g_CollectionSize); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = certCount + 1; // +1 for the trust anchor cleanup: - (*env)->DeleteLocalRef(env, certPathList); + RELEASE_LOCALS(loc, env); return ret; } @@ -202,12 +203,12 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, int32_t ret = FAIL; int certCount = 0; int32_t i = 0; - jobject trustedCert = NULL; + INIT_LOCALS(loc, certPathList, trustedCert); // List certPathList = certPath.getCertificates(); - jobject certPathList = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); + loc[certPathList] = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - certCount = (int)(*env)->CallIntMethod(env, certPathList, g_CollectionSize); + certCount = (int)(*env)->CallIntMethod(env, loc[certPathList], g_CollectionSize); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (certsLen < certCount + 1) goto cleanup; @@ -220,18 +221,18 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, // } for (i = 0; i < certCount; ++i) { - jobject cert = (*env)->CallObjectMethod(env, certPathList, g_ListGet, i); + jobject cert = (*env)->CallObjectMethod(env, loc[certPathList], g_ListGet, i); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); certs[i] = ToGRef(env, cert); } // Certificate trustedCert = trustAnchor.getTrustedCert(); // certs[i] = trustedCert; - trustedCert = (*env)->CallObjectMethod(env, ctx->trustAnchor, g_TrustAnchorGetTrustedCert); + loc[trustedCert] = (*env)->CallObjectMethod(env, ctx->trustAnchor, g_TrustAnchorGetTrustedCert); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (i == 0 || !(*env)->IsSameObject(env, certs[i-1], trustedCert)) + if (i == 0 || !(*env)->IsSameObject(env, certs[i-1], loc[trustedCert])) { - certs[i] = ToGRef(env, trustedCert); + certs[i] = AddGRef(env, loc[trustedCert]); ret = i + 1; } else @@ -241,7 +242,7 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, } cleanup: - (*env)->DeleteLocalRef(env, certPathList); + RELEASE_LOCALS(loc, env); return ret; } @@ -346,8 +347,7 @@ static int32_t PopulateValidationError(JNIEnv* env, jobject error, bool isRevoca int index = -1; PAL_X509ChainStatusFlags chainStatus = PAL_X509ChainNoError; uint16_t* messagePtr = NULL; - jobject reason = NULL; - jobject message = NULL; + INIT_LOCALS(loc, reason, message); if ((*env)->IsInstanceOf(env, error, g_CertPathValidatorExceptionClass)) { @@ -355,25 +355,25 @@ static int32_t PopulateValidationError(JNIEnv* env, jobject error, bool isRevoca ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // Get the reason and convert it to a chain status flag. - reason = (*env)->CallObjectMethod(env, error, g_CertPathValidatorExceptionGetReason); + loc[reason] = (*env)->CallObjectMethod(env, error, g_CertPathValidatorExceptionGetReason); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - chainStatus = ChainStatusFromValidatorExceptionReason(env, reason); + chainStatus = ChainStatusFromValidatorExceptionReason(env, loc[reason]); } else { chainStatus = isRevocationError ? PAL_X509ChainRevocationStatusUnknown : PAL_X509ChainPartialChain; } - message = (*env)->CallObjectMethod(env, error, g_ThrowableGetMessage); + loc[message] = (*env)->CallObjectMethod(env, error, g_ThrowableGetMessage); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (message != NULL) + if (loc[message] != NULL) { - jsize messageLen = (*env)->GetStringLength(env, message); + jsize messageLen = (*env)->GetStringLength(env, loc[message]); // +1 for null terminator messagePtr = xmalloc(sizeof(uint16_t) * (size_t)(messageLen + 1)); messagePtr[messageLen] = '\0'; - (*env)->GetStringRegion(env, message, 0, messageLen, (jchar*)messagePtr); + (*env)->GetStringRegion(env, loc[message], 0, messageLen, (jchar*)messagePtr); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } @@ -392,8 +392,7 @@ static int32_t PopulateValidationError(JNIEnv* env, jobject error, bool isRevoca cleanup: free(messagePtr); - (*env)->DeleteLocalRef(env, reason); - (*env)->DeleteLocalRef(env, message); + RELEASE_LOCALS(loc, env); return ret; } @@ -478,30 +477,31 @@ int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainContext* ctx, JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + INIT_LOCALS(loc, anchors); // HashSet anchors = new HashSet(customTrustStoreLen); // for (Certificate cert : customTrustStore) { // TrustAnchor anchor = new TrustAnchor(cert, null); // anchors.Add(anchor); // } - jobject anchors = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, customTrustStoreLen); + loc[anchors] = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, customTrustStoreLen); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); for (int i = 0; i < customTrustStoreLen; ++i) { jobject anchor = (*env)->NewObject(env, g_TrustAnchorClass, g_TrustAnchorCtor, customTrustStore[i], NULL); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - (*env)->CallBooleanMethod(env, anchors, g_HashSetAdd, anchor); + (*env)->CallBooleanMethod(env, loc[anchors], g_HashSetAdd, anchor); (*env)->DeleteLocalRef(env, anchor); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } // params.setTrustAnchors(anchors); - (*env)->CallVoidMethod(env, ctx->params, g_PKIXBuilderParametersSetTrustAnchors, anchors); + (*env)->CallVoidMethod(env, ctx->params, g_PKIXBuilderParametersSetTrustAnchors, loc[anchors]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = SUCCESS; cleanup: - (*env)->DeleteLocalRef(env, anchors); + RELEASE_LOCALS(loc, env); return ret; } From 513ce85ce9391d7a2a686e84b2fc70f055712fc4 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Sun, 12 Jul 2026 00:06:29 +0200 Subject: [PATCH 3/6] [Android] Make X509ChainGetCertificates populate the output array all-or-nothing `AndroidCryptoNative_X509ChainGetCertificates` filled the caller's `certs` array incrementally and, if a JNI call threw partway through, left the already-created global references in the array and returned FAIL, relying on the managed wrapper's `finally` to release those partial entries. Track the number of entries stored and, on any failure, release the global references we created and clear the slots, so the native function is self-contained: the caller only ever observes a fully populated array on success and never has to clean up partial native state on failure. Update the managed wrapper comment accordingly (its `finally` now releases the native-returned refs on success and is a no-op on failure). Native build (android-arm64 Release) is clean with 0 warnings / 0 errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bc96caa6-f528-4662-8cfa-df04e8a47475 --- .../Interop.X509Chain.cs | 6 ++-- .../pal_x509chain.c | 30 ++++++++++++------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs index 89251c82df05c1..863da7370d847b 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.X509Chain.cs @@ -72,9 +72,9 @@ internal static X509Certificate2[] X509ChainGetCertificates(SafeX509ChainContext } finally { - // The native side can populate part of certPtrs and then fail (returning 0) if a JNI - // exception is thrown mid-loop, so release every non-null entry rather than only the - // first `res` entries. + // X509Certificate2 above duplicates each global ref, so release the native-returned + // refs here. On failure the native side already released and cleared its entries, so + // the surviving non-null entries are the ones we still own. for (int i = 0; i < certPtrs.Length; i++) { if (certPtrs[i] != IntPtr.Zero) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c index 358923988e19a0..35cf10177890ad 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c @@ -202,7 +202,7 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, int32_t ret = FAIL; int certCount = 0; - int32_t i = 0; + int32_t added = 0; INIT_LOCALS(loc, certPathList, trustedCert); // List certPathList = certPath.getCertificates(); @@ -215,33 +215,41 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, abort_if_invalid_pointer_argument (certs); + // Populate `certs`, tracking how many entries we've stored (`added`) so that any failure + // can roll them back -- the caller must never observe a partially populated array. // for (int i = 0; i < certPathList.size(); ++i) { // Certificate cert = certPathList.get(i); // certs[i] = cert; // } - for (i = 0; i < certCount; ++i) + for (int32_t i = 0; i < certCount; ++i) { jobject cert = (*env)->CallObjectMethod(env, loc[certPathList], g_ListGet, i); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - certs[i] = ToGRef(env, cert); + certs[added++] = ToGRef(env, cert); } // Certificate trustedCert = trustAnchor.getTrustedCert(); // certs[i] = trustedCert; loc[trustedCert] = (*env)->CallObjectMethod(env, ctx->trustAnchor, g_TrustAnchorGetTrustedCert); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (i == 0 || !(*env)->IsSameObject(env, certs[i-1], loc[trustedCert])) + if (added == 0 || !(*env)->IsSameObject(env, certs[added - 1], loc[trustedCert])) { - certs[i] = AddGRef(env, loc[trustedCert]); - ret = i + 1; - } - else - { - ret = i; - certs[i] = NULL; + certs[added++] = AddGRef(env, loc[trustedCert]); } + ret = added; + cleanup: + if (ret == FAIL) + { + // Release any global refs we already stored and clear the slots so the caller never + // observes a partially populated array when we fail partway through. + for (int32_t i = 0; i < added; ++i) + { + ReleaseGRef(env, certs[i]); + certs[i] = NULL; + } + } RELEASE_LOCALS(loc, env); return ret; } From 5dae3778799573b8d050ad59c1d37f8a244f4f96 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 16 Jul 2026 12:05:48 +0200 Subject: [PATCH 4/6] [Android] Address review feedback: leak-safe JNI local ref handling - pal_misc: check for a pending JNI exception immediately after SetByteArrayRegion in GetRandomBytes - pal_x509chain: move exception checks ahead of DeleteLocalRef, and store loop/temporary local refs (anchor, endOnly, cert) in the loc[] array so RELEASE_LOCALS frees them on every path; null out message after free in the X509ChainGetErrors failure rollback - pal_sslstream: use INIT_LOCALS/RELEASE_LOCALS in SSLStreamRead instead of a raw local + manual ReleaseLRef - pal_eckey: convert NewEcKeyFromKeys to the INIT_LOCALS/RELEASE_LOCALS cleanup pattern to avoid leaking the curveParameters local ref - Interop.Bignum: reword the negative-result comment more conservatively Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1f935e1-2a62-4d1b-94ed-88a01eafaa4b --- .../Interop.Bignum.cs | 5 ++-- .../pal_eckey.c | 17 +++++++++---- .../pal_misc.c | 1 + .../pal_sslstream.c | 10 ++++---- .../pal_x509chain.c | 24 ++++++++++--------- 5 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs index 52f94f6888ddba..8c2fb821d897b9 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Bignum.cs @@ -42,8 +42,9 @@ internal static partial class Crypto written = BigNumToBinary(bignum, start); } - // A negative result means a pending JNI exception was cleared and no data was written. - // Fail loudly rather than silently returning a zero-filled (corrupt) buffer. + // A negative result means the native side cleared a pending JNI exception, so the + // output must be treated as invalid. Fail loudly rather than returning a buffer that + // may be zero-filled or only partially written. if (written < 0) throw new CryptographicException(); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c index 75b296d28cb4f1..d1b1271d00a9f8 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_eckey.c @@ -22,14 +22,21 @@ EC_KEY* AndroidCryptoNative_NewEcKeyFromKeys(JNIEnv *env, jobject /*ECPublicKey* { abort_if_invalid_pointer_argument (publicKey); + EC_KEY* ret = NULL; + INIT_LOCALS(loc, curveParameters); + if (!(*env)->IsInstanceOf(env, publicKey, g_ECPublicKeyClass)) - return NULL; + goto cleanup; + + loc[curveParameters] = (*env)->CallObjectMethod(env, publicKey, g_ECPublicKeyGetParams); + if (CheckJNIExceptions(env) || loc[curveParameters] == NULL) + goto cleanup; - jobject curveParameters = (*env)->CallObjectMethod(env, publicKey, g_ECPublicKeyGetParams); - if (CheckJNIExceptions(env) || curveParameters == NULL) - return NULL; + ret = AndroidCryptoNative_NewEcKey(AddGRef(env, loc[curveParameters]), AndroidCryptoNative_CreateKeyPair(env, publicKey, privateKey)); - return AndroidCryptoNative_NewEcKey(ToGRef(env, curveParameters), AndroidCryptoNative_CreateKeyPair(env, publicKey, privateKey)); +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } #pragma clang diagnostic push diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c index fb08ce5619a5ff..aefb20db7b5a2f 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_misc.c @@ -23,6 +23,7 @@ int32_t CryptoNative_GetRandomBytes(uint8_t* buff, int32_t len) loc[buffArray] = make_java_byte_array(env, len); (*env)->SetByteArrayRegion(env, loc[buffArray], 0, len, (jbyte*)buff); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, loc[randObj], g_randNextBytesMethod, loc[buffArray]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->GetByteArrayRegion(env, loc[buffArray], 0, len, (jbyte*)buff); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c index 1672b0145f4144..4d0375154d297c 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c @@ -823,10 +823,10 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t abort_if_invalid_pointer_argument (sslStream); abort_if_invalid_pointer_argument (read); - jbyteArray data = NULL; JNIEnv* env = GetJNIEnv(); PAL_SSLStreamStatus ret = SSLStreamStatus_Error; *read = 0; + INIT_LOCALS(loc, dataArray); /* appInBuffer.flip(); @@ -878,12 +878,12 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t if (rem > 0) { int32_t bytes_to_read = rem < length ? rem : length; - data = make_java_byte_array(env, bytes_to_read); - IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferGet, data)); + loc[dataArray] = make_java_byte_array(env, bytes_to_read); + IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferGet, loc[dataArray])); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferCompact)); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - (*env)->GetByteArrayRegion(env, data, 0, bytes_to_read, (jbyte*)buffer); + (*env)->GetByteArrayRegion(env, loc[dataArray], 0, bytes_to_read, (jbyte*)buffer); *read = bytes_to_read; ret = SSLStreamStatus_OK; } @@ -893,7 +893,7 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t } cleanup: - ReleaseLRef(env, data); + RELEASE_LOCALS(loc, env); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c index 35cf10177890ad..7ea975ce991bc0 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c @@ -203,7 +203,7 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, int32_t ret = FAIL; int certCount = 0; int32_t added = 0; - INIT_LOCALS(loc, certPathList, trustedCert); + INIT_LOCALS(loc, certPathList, trustedCert, cert); // List certPathList = certPath.getCertificates(); loc[certPathList] = (*env)->CallObjectMethod(env, ctx->certPath, g_CertPathGetCertificates); @@ -223,9 +223,10 @@ int32_t AndroidCryptoNative_X509ChainGetCertificates(X509ChainContext* ctx, // } for (int32_t i = 0; i < certCount; ++i) { - jobject cert = (*env)->CallObjectMethod(env, loc[certPathList], g_ListGet, i); + loc[cert] = (*env)->CallObjectMethod(env, loc[certPathList], g_ListGet, i); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - certs[added++] = ToGRef(env, cert); + certs[added++] = ToGRef(env, loc[cert]); + loc[cert] = NULL; } // Certificate trustedCert = trustAnchor.getTrustedCert(); @@ -469,6 +470,7 @@ int32_t AndroidCryptoNative_X509ChainGetErrors(X509ChainContext* ctx, Validation for (int32_t i = 0; i < populated; ++i) { free(errors[i].message); + errors[i].message = NULL; } } return ret; @@ -485,7 +487,7 @@ int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainContext* ctx, JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - INIT_LOCALS(loc, anchors); + INIT_LOCALS(loc, anchors, anchor); // HashSet anchors = new HashSet(customTrustStoreLen); // for (Certificate cert : customTrustStore) { @@ -496,11 +498,12 @@ int32_t AndroidCryptoNative_X509ChainSetCustomTrustStore(X509ChainContext* ctx, ON_EXCEPTION_PRINT_AND_GOTO(cleanup); for (int i = 0; i < customTrustStoreLen; ++i) { - jobject anchor = (*env)->NewObject(env, g_TrustAnchorClass, g_TrustAnchorCtor, customTrustStore[i], NULL); + loc[anchor] = (*env)->NewObject(env, g_TrustAnchorClass, g_TrustAnchorCtor, customTrustStore[i], NULL); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - (*env)->CallBooleanMethod(env, loc[anchors], g_HashSetAdd, anchor); - (*env)->DeleteLocalRef(env, anchor); + (*env)->CallBooleanMethod(env, loc[anchors], g_HashSetAdd, loc[anchor]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ReleaseLRef(env, loc[anchor]); + loc[anchor] = NULL; } // params.setTrustAnchors(anchors); @@ -557,7 +560,7 @@ static int32_t ValidateWithRevocation(JNIEnv* env, abort_if_invalid_pointer_argument (validator); int32_t ret = FAIL; - INIT_LOCALS(loc, certPathList, certPathFromAnchor, options, checker, result, ex, revocationErrorList); + INIT_LOCALS(loc, certPathList, certPathFromAnchor, options, checker, result, ex, revocationErrorList, endOnly); if (revocationMode == X509RevocationMode_Offline) { @@ -602,10 +605,9 @@ static int32_t ValidateWithRevocation(JNIEnv* env, // options.add(PKIXRevocationChecker.Option.ONLY_END_ENTITY); loc[options] = (*env)->NewObject(env, g_HashSetClass, g_HashSetCtorWithCapacity, 3); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - jobject endOnly = (*env)->GetStaticObjectField( + loc[endOnly] = (*env)->GetStaticObjectField( env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionOnlyEndEntity); - (*env)->CallBooleanMethod(env, loc[options], g_HashSetAdd, endOnly); - (*env)->DeleteLocalRef(env, endOnly); + (*env)->CallBooleanMethod(env, loc[options], g_HashSetAdd, loc[endOnly]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } } From 5700cf0e8e5d605ff7016d5a34e64fb629d51d7f Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 16 Jul 2026 12:18:28 +0200 Subject: [PATCH 5/6] [Android] Add JNI exception checks after GetStaticObjectField and GetArrayLength Address review feedback: - pal_x509chain (ValidateWithRevocation): check for a pending JNI exception immediately after GetStaticObjectField for the ONLY_END_ENTITY option, before calling CallBooleanMethod - pal_hmac (DoFinal): check after GetArrayLength so we don't call GetByteArrayRegion with a pending exception Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e1f935e1-2a62-4d1b-94ed-88a01eafaa4b --- .../libs/System.Security.Cryptography.Native.Android/pal_hmac.c | 1 + .../System.Security.Cryptography.Native.Android/pal_x509chain.c | 1 + 2 files changed, 2 insertions(+) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c index 69bdaa2c189538..d5927cd8de33bf 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_hmac.c @@ -107,6 +107,7 @@ ARGS_NON_NULL_ALL static int32_t DoFinal(JNIEnv* env, jobject mac, uint8_t* data loc[dataBytes] = (jbyteArray)(*env)->CallObjectMethod(env, mac, g_MacDoFinal); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); dataBytesLen = (*env)->GetArrayLength(env, loc[dataBytes]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->GetByteArrayRegion(env, loc[dataBytes], 0, dataBytesLen, (jbyte*) data); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *len = (int32_t)dataBytesLen; diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c index 7ea975ce991bc0..04e6a4c5614a45 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c @@ -607,6 +607,7 @@ static int32_t ValidateWithRevocation(JNIEnv* env, ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[endOnly] = (*env)->GetStaticObjectField( env, g_PKIXRevocationCheckerOptionClass, g_PKIXRevocationCheckerOptionOnlyEndEntity); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallBooleanMethod(env, loc[options], g_HashSetAdd, loc[endOnly]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } From ac29ec00e176753063af7a6ab40c0b2485b38e69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Rozs=C3=ADval?= Date: Thu, 16 Jul 2026 21:56:27 +0200 Subject: [PATCH 6/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../System.Security.Cryptography.Native.Android/pal_sslstream.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c index 4d0375154d297c..ca928842af147a 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_sslstream.c @@ -884,6 +884,7 @@ AndroidCryptoNative_SSLStreamRead(SSLStream* sslStream, uint8_t* buffer, int32_t IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appInBuffer, g_ByteBufferCompact)); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->GetByteArrayRegion(env, loc[dataArray], 0, bytes_to_read, (jbyte*)buffer); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *read = bytes_to_read; ret = SSLStreamStatus_OK; }