From 6f97792e5b84aa9789602cb0a3a54a5fa0ddc817 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 29 May 2026 13:09:39 +0200 Subject: [PATCH 01/14] [Android] Add missing JNI exception checks across crypto PAL (1/3) This is PR 1 of 3 addressing findings from a static audit of the Android crypto PAL (libSystem.Security.Cryptography.Native.Android). It targets the 26 HIGH-severity findings on files that are not touched by any other in-flight PR. Two follow-up PRs will address the remaining HIGH findings on pal_x509chain.c and pal_x509store.c (after merging dependencies) and the MEDIUM/LOW findings. Files affected (10): pal_cipher.c (1 HIGH) pal_dsa.c (4 HIGH) pal_ecc_import_export.c (8 HIGH) pal_ecdh.c (1 HIGH) pal_eckey.c (1 HIGH) pal_hmac.c (1 HIGH + a gref leak on failure path) pal_ssl.c (3 HIGH) pal_sslstream.c (5 HIGH + 2 tightly coupled bug fixes) pal_x509.c (2 HIGH) pal_x509store.c (4 HIGH + 1 partial) Patterns applied ---------------- - Pattern B (chained Call*Method without intermediate checks): inserted ON_EXCEPTION_PRINT_AND_GOTO between successive throwing JNI calls so later calls never run with a pending exception or on a NULL receiver (undefined behavior today, varying by CheckJNI vs. release). - Pattern C (pending exception leaks back to .NET): added explicit CheckJNIExceptions or TryClearJNIExceptions at the relevant return paths so callers see a clean FAIL rather than a poisoned env. - Internal helpers ContainsEntryForAlias and ContainsMatchingCertificateForAlias in pal_x509store.c were refactored to return int32_t with an out-param so JNI exceptions can no longer be swallowed by collapsing them into a bool result. Tightly coupled bug fixes in pal_sslstream.c -------------------------------------------- - FreeSSLStream: NULL-check managedContextCleanup before calling it. This crash is reachable today on main: when InitializeSslContext throws (e.g. EncryptionPolicy.NoEncryption), Dispose runs on a zero-initialized SSLStream and dereferences a NULL function pointer. Verified by running System.Net.Security.Tests against pristine main: same SIGSEGV at AndroidCryptoNative_SSLStreamRelease+48. - SSLStreamInitialize: hoist managedContext / streamReader / streamWriter / managedContextCleanup assignments above the first goto so a failure during init leaves the struct in a consistent state for FreeSSLStream. Out of scope ------------ - Explicit abort_unless / abort_if_invalid_pointer_argument on JNI pointers in pal_evp.c, pal_ecdsa.c, and pal_dsa.c:192 are left as-is per the directive to preserve existing predictable abort behavior. Validation ---------- - ./build.sh libs.native -os android -arch arm64 -c Debug : OK - ./build.sh clr+libs+host -rc release -lc release : OK (baseline) - System.Security.Cryptography.Tests on emulator-5554 : Tests run: 11737 Passed: 10738 Failed: 0 Ignored: 658 Skipped: 977 - System.Net.Security.Tests on R58Y30HZ65V (API 36) : Tests run: 5124 Passed: 4854 Failed: 4 Ignored: 234 Skipped: 32 The four failures are pre-existing on main (verified by re-running the same tests against a pristine main native lib): two CertificateSelectionCallback_DelayedCertificate_OK cases that report SkipTestException as FAIL, and two AlpnListTotalSizeExceedsLimit_Throws cases where Android wraps ArgumentException in AuthenticationException. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pal_cipher.c | 9 ++ .../pal_dsa.c | 17 ++- .../pal_ecc_import_export.c | 76 ++++++++++++- .../pal_ecdh.c | 5 + .../pal_eckey.c | 7 ++ .../pal_hmac.c | 17 ++- .../pal_ssl.c | 4 + .../pal_sslstream.c | 44 ++++++-- .../pal_x509.c | 2 + .../pal_x509store.c | 100 ++++++++++++++---- 10 files changed, 249 insertions(+), 32 deletions(-) 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 a775b0f7552888..8e1e58624a52e4 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 @@ -168,6 +168,15 @@ ARGS_NON_NULL_ALL static int32_t ReinitializeCipher(CipherCtx* ctx) } (*env)->DeleteLocalRef(env, ivBytes); + + if (CheckJNIExceptions(env)) + { + (*env)->DeleteLocalRef(env, algName); + (*env)->DeleteLocalRef(env, sksObj); + (*env)->DeleteLocalRef(env, ivPsObj); + (*env)->DeleteLocalRef(env, keyBytes); + return FAIL; + } } (*env)->CallVoidMethod(env, ctx->cipher, g_cipherInitMethod, ctx->encMode, sksObj, ivPsObj); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c index e18e87515b05e1..09325ad4b7925a 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c @@ -47,7 +47,9 @@ ARGS_NON_NULL_ALL static jobject GetQParameter(JNIEnv* env, jobject dsa) INIT_LOCALS(loc, algName, keyFactory, publicKey, publicKeySpec); loc[algName] = make_java_string(env, "DSA"); loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[algName]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[publicKey] = (*env)->CallObjectMethod(env, dsa, g_keyPairGetPublicMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[publicKeySpec] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[publicKey], g_DSAPublicKeySpecClass); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); @@ -93,7 +95,9 @@ int32_t AndroidCryptoNative_DsaSizeP(jobject dsa) INIT_LOCALS(loc, algName, keyFactory, publicKey, publicKeySpec, p); loc[algName] = make_java_string(env, "DSA"); loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[algName]); + ON_EXCEPTION_PRINT_AND_GOTO(error); loc[publicKey] = (*env)->CallObjectMethod(env, dsa, g_keyPairGetPublicMethod); + ON_EXCEPTION_PRINT_AND_GOTO(error); loc[publicKeySpec] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[publicKey], g_DSAPublicKeySpecClass); ON_EXCEPTION_PRINT_AND_GOTO(error); @@ -222,14 +226,20 @@ int32_t AndroidCryptoNative_GetDsaParameters( loc[algName] = make_java_string(env, "DSA"); loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[algName]); + ON_EXCEPTION_PRINT_AND_GOTO(error); loc[publicKey] = (*env)->CallObjectMethod(env, dsa, g_keyPairGetPublicMethod); + ON_EXCEPTION_PRINT_AND_GOTO(error); loc[publicKeySpec] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[publicKey], g_DSAPublicKeySpecClass); ON_EXCEPTION_PRINT_AND_GOTO(error); *p = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetP)); + ON_EXCEPTION_PRINT_AND_GOTO(error); *q = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetQ)); + ON_EXCEPTION_PRINT_AND_GOTO(error); *g = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetG)); + ON_EXCEPTION_PRINT_AND_GOTO(error); *y = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetY)); + ON_EXCEPTION_PRINT_AND_GOTO(error); *pLength = AndroidCryptoNative_GetBigNumBytes(*p); *qLength = AndroidCryptoNative_GetBigNumBytes(*q); *gLength = AndroidCryptoNative_GetBigNumBytes(*g); @@ -238,16 +248,18 @@ int32_t AndroidCryptoNative_GetDsaParameters( *x = NULL; *xLength = 0; loc[privateKey] = (*env)->CallObjectMethod(env, dsa, g_keyPairGetPrivateMethod); + ON_EXCEPTION_PRINT_AND_GOTO(error); if (loc[privateKey]) { loc[privateKeySpec] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[privateKey], g_DSAPrivateKeySpecClass); ON_EXCEPTION_PRINT_AND_GOTO(error); *x = ToGRef(env, (*env)->CallObjectMethod(env, loc[privateKeySpec], g_DSAPrivateKeySpecGetX)); + ON_EXCEPTION_PRINT_AND_GOTO(error); *xLength = AndroidCryptoNative_GetBigNumBytes(*x); } RELEASE_LOCALS_ENV(loc, ReleaseLRef); - return CheckJNIExceptions(env) ? FAIL : SUCCESS; + return SUCCESS; error: RELEASE_LOCALS_ENV(loc, ReleaseLRef); @@ -279,15 +291,18 @@ int32_t AndroidCryptoNative_DsaKeyCreateByExplicitParameters( bn[G] = AndroidCryptoNative_BigNumFromBinary(g, gLength); bn[Y] = AndroidCryptoNative_BigNumFromBinary(y, yLength); loc[publicKeySpec] = (*env)->NewObject(env, g_DSAPublicKeySpecClass, g_DSAPublicKeySpecCtor, bn[Y], bn[P], bn[Q], bn[G]); + ON_EXCEPTION_PRINT_AND_GOTO(error); if (x) { bn[X] = AndroidCryptoNative_BigNumFromBinary(x, xLength); loc[privateKeySpec] = (*env)->NewObject(env, g_DSAPrivateKeySpecClass, g_DSAPrivateKeySpecCtor, bn[X], bn[P], bn[Q], bn[G]); + ON_EXCEPTION_PRINT_AND_GOTO(error); } loc[dsa] = make_java_string(env, "DSA"); loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[dsa]); + ON_EXCEPTION_PRINT_AND_GOTO(error); loc[publicKey] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGenPublicMethod, loc[publicKeySpec]); ON_EXCEPTION_PRINT_AND_GOTO(error); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c index 6b18eddcad17da..b9ea77d974472e 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c @@ -28,16 +28,33 @@ int32_t AndroidCryptoNative_GetECKeyParameters(const EC_KEY* key, // Get the public key jobject publicKey = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPublicMethod); + if (CheckJNIExceptions(env)) + goto error; jobject Q = (*env)->CallObjectMethod(env, publicKey, g_ECPublicKeyGetW); (*env)->DeleteLocalRef(env, publicKey); + if (CheckJNIExceptions(env)) + goto error; + jobject xBn = (*env)->CallObjectMethod(env, Q, g_ECPointGetAffineX); + if (CheckJNIExceptions(env)) + { + (*env)->DeleteLocalRef(env, Q); + goto error; + } jobject yBn = (*env)->CallObjectMethod(env, Q, g_ECPointGetAffineY); (*env)->DeleteLocalRef(env, Q); + if (CheckJNIExceptions(env)) + { + if (xBn) (*env)->DeleteLocalRef(env, xBn); + if (yBn) (*env)->DeleteLocalRef(env, yBn); + goto error; + } + // Success; assign variables *qx = ToGRef(env, xBn); *cbQx = AndroidCryptoNative_GetBigNumBytes(*qx); @@ -53,6 +70,12 @@ int32_t AndroidCryptoNative_GetECKeyParameters(const EC_KEY* key, abort_if_invalid_pointer_argument (d); jobject privateKey = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPrivateMethod); + if (CheckJNIExceptions(env)) + { + *d = NULL; + *cbD = 0; + goto error; + } if (!privateKey) { @@ -65,6 +88,13 @@ int32_t AndroidCryptoNative_GetECKeyParameters(const EC_KEY* key, (*env)->DeleteLocalRef(env, privateKey); + if (CheckJNIExceptions(env)) + { + *d = NULL; + *cbD = 0; + goto error; + } + *d = ToGRef(env, dBn); *cbD = AndroidCryptoNative_GetBigNumBytes(*d); if (*cbD == FAIL) @@ -148,16 +178,21 @@ int32_t AndroidCryptoNative_GetECCurveParameters(const EC_KEY* key, goto error; loc[group] = (*env)->CallObjectMethod(env, key->curveParameters, g_ECParameterSpecGetCurve); + ON_EXCEPTION_PRINT_AND_GOTO(error); bn[A] = (*env)->CallObjectMethod(env, loc[group], g_EllipticCurveGetA); + ON_EXCEPTION_PRINT_AND_GOTO(error); bn[B] = (*env)->CallObjectMethod(env, loc[group], g_EllipticCurveGetB); + ON_EXCEPTION_PRINT_AND_GOTO(error); loc[field] = (*env)->CallObjectMethod(env, loc[group], g_EllipticCurveGetField); + ON_EXCEPTION_PRINT_AND_GOTO(error); if ((*env)->IsInstanceOf(env, loc[field], g_ECFieldF2mClass)) { *curveType = Characteristic2; // Get the reduction polynomial p bn[P] = (*env)->CallObjectMethod(env, loc[field], g_ECFieldF2mGetReductionPolynomial); + ON_EXCEPTION_PRINT_AND_GOTO(error); } else { @@ -165,26 +200,35 @@ int32_t AndroidCryptoNative_GetECCurveParameters(const EC_KEY* key, *curveType = PrimeShortWeierstrass; // Get the prime p bn[P] = (*env)->CallObjectMethod(env, loc[field], g_ECFieldFpGetP); + ON_EXCEPTION_PRINT_AND_GOTO(error); } // Extract gx and gy loc[G] = (*env)->CallObjectMethod(env, key->curveParameters, g_ECParameterSpecGetGenerator); + ON_EXCEPTION_PRINT_AND_GOTO(error); bn[X] = (*env)->CallObjectMethod(env, loc[G], g_ECPointGetAffineX); + ON_EXCEPTION_PRINT_AND_GOTO(error); bn[Y] = (*env)->CallObjectMethod(env, loc[G], g_ECPointGetAffineY); + ON_EXCEPTION_PRINT_AND_GOTO(error); // Extract order (n) bn[ORDER] = (*env)->CallObjectMethod(env, key->curveParameters, g_ECParameterSpecGetOrder); + ON_EXCEPTION_PRINT_AND_GOTO(error); // Extract cofactor (h) int32_t cofactorInt = (*env)->CallIntMethod(env, key->curveParameters, g_ECParameterSpecGetCofactor); + ON_EXCEPTION_PRINT_AND_GOTO(error); bn[COFACTOR] = (*env)->CallStaticObjectMethod(env, g_bigNumClass, g_valueOfMethod, (int64_t)cofactorInt); + ON_EXCEPTION_PRINT_AND_GOTO(error); // Extract seed (optional) loc[seedArray] = (*env)->CallObjectMethod(env, loc[group], g_EllipticCurveGetSeed); + ON_EXCEPTION_PRINT_AND_GOTO(error); if (loc[seedArray]) { bn[SEED] = (*env)->NewObject(env, g_bigNumClass, g_bigNumCtorWithSign, 1, loc[seedArray]); + ON_EXCEPTION_PRINT_AND_GOTO(error); *seed = ToGRef(env, bn[SEED]); *cbSeed = AndroidCryptoNative_GetBigNumBytes(*seed); @@ -270,8 +314,10 @@ static jobject CreateKeyPairFromCurveParameters( goto error; loc[pubKeyPoint] = (*env)->NewObject(env, g_ECPointClass, g_ECPointCtor, bn[QX], bn[QY]); + ON_EXCEPTION_PRINT_AND_GOTO(error); loc[pubKeySpec] = (*env)->NewObject(env, g_ECPublicKeySpecClass, g_ECPublicKeySpecCtor, loc[pubKeyPoint], curveParameters); + ON_EXCEPTION_PRINT_AND_GOTO(error); // Set private key (optional) if (d && dLength) @@ -281,6 +327,7 @@ static jobject CreateKeyPairFromCurveParameters( goto error; loc[privKeySpec] = (*env)->NewObject(env, g_ECPrivateKeySpecClass, g_ECPrivateKeySpecCtor, bn[D], curveParameters); + ON_EXCEPTION_PRINT_AND_GOTO(error); } } // If we don't have the public key but we have the private key, we can @@ -292,6 +339,7 @@ static jobject CreateKeyPairFromCurveParameters( goto error; loc[privKeySpec] = (*env)->NewObject(env, g_ECPrivateKeySpecClass, g_ECPrivateKeySpecCtor, bn[D], curveParameters); + ON_EXCEPTION_PRINT_AND_GOTO(error); // Java doesn't have a public implementation of operations on points on an elliptic curve // so we can't yet derive a new public key from the private key and generator. @@ -306,6 +354,7 @@ static jobject CreateKeyPairFromCurveParameters( // Create the private and public keys and put them into a key pair. loc[algorithmName] = make_java_string(env, "EC"); loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[algorithmName]); + ON_EXCEPTION_PRINT_AND_GOTO(error); loc[publicKey] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGenPublicMethod, loc[pubKeySpec]); ON_EXCEPTION_PRINT_AND_GOTO(error); @@ -374,18 +423,27 @@ int32_t AndroidCryptoNative_EcKeyCreateByKeyParameters(EC_KEY** key, } // Converts a java.math.BigInteger to a positive int32_t value. -// Returns -1 if bigInteger < 0 or > INT32_MAX +// Returns -1 if bigInteger < 0 or > INT32_MAX, or if a pending JNI exception is detected. ARGS_NON_NULL_ALL static int32_t ConvertBigIntegerToPositiveInt32(JNIEnv* env, jobject bigInteger) { + jint signum = (*env)->CallIntMethod(env, bigInteger, g_sigNumMethod); + if (CheckJNIExceptions(env)) + return -1; + // bigInteger is negative. - if ((*env)->CallIntMethod(env, bigInteger, g_sigNumMethod) < 0) + if (signum < 0) { return -1; } jobject int32MaxBigInt = (*env)->CallStaticObjectMethod(env, g_bigNumClass, g_valueOfMethod, (int64_t)INT32_MAX); + if (CheckJNIExceptions(env)) + return -1; + int willOverflow = (*env)->CallIntMethod(env, bigInteger, g_compareToMethod, int32MaxBigInt); (*env)->DeleteLocalRef(env, int32MaxBigInt); + if (CheckJNIExceptions(env)) + return -1; // If bigInteger > int32MaxBigInt, then a conversion to int32_t would be lossy. if (willOverflow > 0) @@ -393,7 +451,10 @@ ARGS_NON_NULL_ALL static int32_t ConvertBigIntegerToPositiveInt32(JNIEnv* env, j return -1; } - return (*env)->CallIntMethod(env, bigInteger, g_intValueMethod); + jint result = (*env)->CallIntMethod(env, bigInteger, g_intValueMethod); + if (CheckJNIExceptions(env)) + return -1; + return result; } EC_KEY* AndroidCryptoNative_EcKeyCreateByExplicitParameters(ECCurveType curveType, @@ -489,16 +550,19 @@ EC_KEY* AndroidCryptoNative_EcKeyCreateByExplicitParameters(ECCurveType curveTyp loc[seedArray] = make_java_byte_array(env, seedLength); (*env)->SetByteArrayRegion(env, loc[seedArray], 0, seedLength, (jbyte*)seed); loc[group] = (*env)->NewObject(env, g_EllipticCurveClass, g_EllipticCurveCtorWithSeed, loc[field], bn[A], bn[B], loc[seedArray]); + ON_EXCEPTION_PRINT_AND_GOTO(error); } else { loc[group] = (*env)->NewObject(env, g_EllipticCurveClass, g_EllipticCurveCtor, loc[field], bn[A], bn[B]); + ON_EXCEPTION_PRINT_AND_GOTO(error); } // Set generator, order and cofactor bn[GX] = AndroidCryptoNative_BigNumFromBinary(gx, gxLength); bn[GY] = AndroidCryptoNative_BigNumFromBinary(gy, gyLength); loc[G] = (*env)->NewObject(env, g_ECPointClass, g_ECPointCtor, bn[GX], bn[GY]); + ON_EXCEPTION_PRINT_AND_GOTO(error); bn[ORDER] = AndroidCryptoNative_BigNumFromBinary(order, orderLength); @@ -514,6 +578,7 @@ EC_KEY* AndroidCryptoNative_EcKeyCreateByExplicitParameters(ECCurveType curveTyp } loc[paramSpec] = (*env)->NewObject(env, g_ECParameterSpecClass, g_ECParameterSpecCtor, loc[group], loc[G], bn[ORDER], cofactorInt); + ON_EXCEPTION_PRINT_AND_GOTO(error); if ((qx && qy) || d) { @@ -526,6 +591,11 @@ EC_KEY* AndroidCryptoNative_EcKeyCreateByExplicitParameters(ECCurveType curveTyp jstring ec = make_java_string(env, "EC"); jobject keyPairGenerator = (*env)->CallStaticObjectMethod(env, g_keyPairGenClass, g_keyPairGenGetInstanceMethod, ec); + if (CheckJNIExceptions(env)) + { + ReleaseLRef(env, ec); + goto error; + } (*env)->CallVoidMethod(env, keyPairGenerator, g_keyPairGenInitializeWithParamsMethod, loc[paramSpec]); if (CheckJNIExceptions(env)) { diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c index 911975077b8647..0fbd35b1044057 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c @@ -19,6 +19,11 @@ int32_t AndroidCryptoNative_EcdhDeriveKey(EC_KEY* ourKey, EC_KEY* peerKey, uint8 jobject keyAgreement = (*env)->CallStaticObjectMethod(env, g_KeyAgreementClass, g_KeyAgreementGetInstance, algorithmName); ReleaseLRef(env, algorithmName); + if (CheckJNIExceptions(env)) + { + *usedBufferLength = 0; + return FAIL; + } jobject privateKey = (*env)->CallObjectMethod(env, ourKey->keyPair, g_keyPairGetPrivateMethod); 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 765d31f9b419bf..a9bb7582016f48 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 @@ -160,6 +160,13 @@ int32_t AndroidCryptoNative_EcKeyGetSize(const EC_KEY* key, int32_t* keySize) ReleaseLRef(env, field); ReleaseLRef(env, curve); + + if (CheckJNIExceptions(env)) + { + *keySize = 0; + return FAIL; + } + 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 a367d567771829..0f939dcfbce61f 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 @@ -61,12 +61,27 @@ jobject CryptoNative_HmacCreate(uint8_t* key, int32_t keyLen, intptr_t type) } jobject macObj = ToGRef(env, (*env)->CallStaticObjectMethod(env, g_MacClass, g_MacGetInstance, macName)); + if (CheckJNIExceptions(env)) + { + ReleaseGRef(env, macObj); + (*env)->DeleteLocalRef(env, keyBytes); + (*env)->DeleteLocalRef(env, sksObj); + (*env)->DeleteLocalRef(env, macName); + return FAIL; + } + (*env)->CallVoidMethod(env, macObj, g_MacInit, sksObj); (*env)->DeleteLocalRef(env, keyBytes); (*env)->DeleteLocalRef(env, sksObj); (*env)->DeleteLocalRef(env, macName); - return CheckJNIExceptions(env) ? FAIL : macObj; + if (CheckJNIExceptions(env)) + { + ReleaseGRef(env, macObj); + return FAIL; + } + + return macObj; } int32_t CryptoNative_HmacReset(jobject ctx) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ssl.c index 29571dd0289bad..80405457af09c6 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ssl.c @@ -13,8 +13,11 @@ PAL_SslProtocol AndroidCryptoNative_SSLGetSupportedProtocols(void) // SSLParameters params = context.getDefaultSSLParameters(); // String[] protocols = params.getProtocols(); loc[context] = (*env)->CallStaticObjectMethod(env, g_sslCtxClass, g_sslCtxGetDefaultMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[params] = (*env)->CallObjectMethod(env, loc[context], g_sslCtxGetDefaultSslParamsMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[protocols] = (*env)->CallObjectMethod(env, loc[params], g_SSLParametersGetProtocols); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); const char tlsv1[] = "TLSv1"; size_t tlsv1Len = (sizeof(tlsv1) / sizeof(*tlsv1)) - 1; @@ -48,6 +51,7 @@ PAL_SslProtocol AndroidCryptoNative_SSLGetSupportedProtocols(void) (*env)->DeleteLocalRef(env, protocol); } +cleanup: RELEASE_LOCALS(loc, env); return supported; } 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 c45e12b1319d6e..d1d7ef43651000 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 @@ -79,6 +79,8 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus Close(JNIEnv* env, SSLStream* sslSt // sslEngine.closeOutbound(); (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineCloseOutbound); + if (CheckJNIExceptions(env)) + return SSLStreamStatus_Error; if (ret != SSLStreamStatus_OK) return ret; @@ -229,9 +231,16 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss // int count = streamReader(tmp, 0, netInBufferLimit); // netInBuffer.put(tmp); // } - if ((*env)->CallIntMethod(env, sslStream->netInBuffer, g_ByteBufferPosition) == 0) + int position = (*env)->CallIntMethod(env, sslStream->netInBuffer, g_ByteBufferPosition); + if (CheckJNIExceptions(env)) + return SSLStreamStatus_Error; + + if (position == 0) { int netInBufferLimit = (*env)->CallIntMethod(env, sslStream->netInBuffer, g_ByteBufferLimit); + if (CheckJNIExceptions(env)) + return SSLStreamStatus_Error; + uint8_t* tmpNative = (uint8_t*)xmalloc((size_t)netInBufferLimit); int count = netInBufferLimit; // todo assert streamReader != 0 ? @@ -250,6 +259,8 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss cleanup: free(tmpNative); (*env)->DeleteLocalRef(env, tmp); + if (CheckJNIExceptions(env)) + return SSLStreamStatus_Error; } // netInBuffer.flip(); @@ -344,7 +355,12 @@ ARGS_NON_NULL_ALL static void FreeSSLStream(JNIEnv* env, SSLStream* sslStream) ReleaseGRef(env, sslStream->netInBuffer); ReleaseGRef(env, sslStream->appInBuffer); - sslStream->managedContextCleanup(sslStream->managedContextHandle); + // managedContextCleanup may be NULL if the SSLStream was created via SSLStreamCreate + // but SSLStreamInitialize was never called (e.g. when managed code fails validation + // before invoking initialize and then disposes the SafeSslHandle). In that case there + // is no managed handle to release. + if (sslStream->managedContextCleanup != NULL) + sslStream->managedContextCleanup(sslStream->managedContextHandle); free(sslStream); } @@ -445,6 +461,7 @@ static int32_t AddCertChainToStore(JNIEnv* env, loc[keyBytes] = make_java_byte_array(env, pkcs8PrivateKeyLen); (*env)->SetByteArrayRegion(env, loc[keyBytes], 0, pkcs8PrivateKeyLen, (jbyte*)pkcs8PrivateKey); loc[keySpec] = (*env)->NewObject(env, g_PKCS8EncodedKeySpec, g_PKCS8EncodedKeySpecCtor, loc[keyBytes]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); switch (algorithm) { @@ -466,7 +483,9 @@ static int32_t AddCertChainToStore(JNIEnv* env, // PrivateKey privateKey = keyFactory.generatePrivate(spec); loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[algorithmName]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[privateKey] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGenPrivateMethod, loc[keySpec]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // X509Certificate[] certArray = new X509Certificate[certsLen]; loc[certArray] = make_java_object_array(env, certsLen, g_X509CertClass, NULL); @@ -595,6 +614,14 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( int32_t ret = FAIL; JNIEnv* env = GetJNIEnv(); + // Assign callback/handle fields up front so that FreeSSLStream() can safely run + // its managedContextCleanup() invocation if any of the JNI-call failure paths + // below jump to 'exit'. + sslStream->managedContextHandle = managedContextHandle; + sslStream->streamReader = streamReader; + sslStream->streamWriter = streamWriter; + sslStream->managedContextCleanup = managedContextCleanup; + jobject sslEngine = NULL; if (peerHost) { @@ -618,11 +645,14 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( // SSLSession sslSession = sslEngine.getSession(); sslStream->sslSession = ToGRef(env, (*env)->CallObjectMethod(env, sslStream->sslEngine, g_SSLEngineGetSession)); + ON_EXCEPTION_PRINT_AND_GOTO(exit); // int applicationBufferSize = sslSession.getApplicationBufferSize(); // int packetBufferSize = sslSession.getPacketBufferSize(); int32_t applicationBufferSize = (*env)->CallIntMethod(env, sslStream->sslSession, g_SSLSessionGetApplicationBufferSize); + ON_EXCEPTION_PRINT_AND_GOTO(exit); int32_t packetBufferSize = (*env)->CallIntMethod(env, sslStream->sslSession, g_SSLSessionGetPacketBufferSize); + ON_EXCEPTION_PRINT_AND_GOTO(exit); // ByteBuffer appInBuffer = ByteBuffer.allocate(Math.max(applicationBufferSize, appBufferSize)); // ByteBuffer appOutBuffer = ByteBuffer.allocate(appBufferSize); @@ -631,17 +661,16 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( int32_t appInBufferSize = applicationBufferSize > appBufferSize ? applicationBufferSize : appBufferSize; sslStream->appInBuffer = ToGRef(env, (*env)->CallStaticObjectMethod(env, g_ByteBuffer, g_ByteBufferAllocate, appInBufferSize)); + ON_EXCEPTION_PRINT_AND_GOTO(exit); sslStream->appOutBuffer = ToGRef(env, (*env)->CallStaticObjectMethod(env, g_ByteBuffer, g_ByteBufferAllocate, appBufferSize)); + ON_EXCEPTION_PRINT_AND_GOTO(exit); sslStream->netOutBuffer = ToGRef(env, (*env)->CallStaticObjectMethod(env, g_ByteBuffer, g_ByteBufferAllocate, packetBufferSize)); + ON_EXCEPTION_PRINT_AND_GOTO(exit); sslStream->netInBuffer = ToGRef(env, (*env)->CallStaticObjectMethod(env, g_ByteBuffer, g_ByteBufferAllocate, packetBufferSize)); - - sslStream->managedContextHandle = managedContextHandle; - sslStream->streamReader = streamReader; - sslStream->streamWriter = streamWriter; - sslStream->managedContextCleanup = managedContextCleanup; + ON_EXCEPTION_PRINT_AND_GOTO(exit); ret = SUCCESS; @@ -795,6 +824,7 @@ PAL_SSLStreamStatus AndroidCryptoNative_SSLStreamWrite(SSLStream* sslStream, uin // appOutBuffer = EnsureRemaining(appOutBuffer, length); // appOutBuffer.put(bufferByteBuffer); IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appOutBuffer, g_ByteBufferCompact)); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); sslStream->appOutBuffer = EnsureRemaining(env, sslStream->appOutBuffer, length); IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appOutBuffer, g_ByteBufferPutBuffer, bufferByteBuffer)); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); 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 99c6e522a203c6..f9c0b50505179a 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 @@ -101,6 +101,7 @@ int32_t AndroidCryptoNative_X509DecodeCollection(const uint8_t* buf, ON_EXCEPTION_PRINT_AND_GOTO(cleanup); jint certCount = (*env)->CallIntMethod(env, loc[certs], g_CollectionSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); bool insufficientBuffer = *outLen < certCount; *outLen = certCount; @@ -164,6 +165,7 @@ int32_t AndroidCryptoNative_X509ExportPkcs7(jobject* /*X509Certificate[]*/ certs // foreach (Certificate cert in certs) // certList.add(cert); loc[certList] = (*env)->NewObject(env, g_ArrayListClass, g_ArrayListCtorWithCapacity, certsLen); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); for (int i = 0; i < certsLen; ++i) { (*env)->CallBooleanMethod(env, loc[certList], g_ArrayListAdd, certs[i]); diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509store.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509store.c index 83ef030694ba70..981f34ff5a0e91 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509store.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509store.c @@ -18,21 +18,29 @@ typedef enum EntryFlags_MatchesCertificate = 4, } EntryFlags; -// Returns whether or not the store contains the specified alias -// If the entry exists, the flags parameter is set based on the contents of the entry -ARGS_NON_NULL_ALL static bool ContainsEntryForAlias( - JNIEnv* env, jobject /*KeyStore*/ store, jobject /*X509Certificate*/ cert, jstring alias, EntryFlags* flags) +// Determines whether the store contains the specified alias, populating *flags +// with the entry's contents when present. +// Returns SUCCESS if the lookup completed (regardless of whether an entry exists), +// FAIL if a JNI exception was encountered while inspecting the store entry. +// On SUCCESS, *contains indicates whether an entry exists for the alias. +ARGS_NON_NULL_ALL static int32_t ContainsEntryForAlias( + JNIEnv* env, jobject /*KeyStore*/ store, jobject /*X509Certificate*/ cert, jstring alias, bool* contains, EntryFlags* flags) { - bool ret = false; + int32_t ret = FAIL; EntryFlags flagsLocal = EntryFlags_None; + bool containsLocal = false; INIT_LOCALS(loc, entry, existingCert); bool containsAlias = (*env)->CallBooleanMethod(env, store, g_KeyStoreContainsAlias, alias); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (!containsAlias) + { + ret = SUCCESS; goto cleanup; + } - ret = true; + containsLocal = true; // KeyStore.Entry entry = store.getEntry(alias, null); // if (entry instanceof KeyStore.PrivateKeyEntry) { @@ -48,42 +56,62 @@ ARGS_NON_NULL_ALL static bool ContainsEntryForAlias( flagsLocal |= EntryFlags_HasCertificate; flagsLocal |= EntryFlags_HasPrivateKey; loc[existingCert] = (*env)->CallObjectMethod(env, loc[entry], g_PrivateKeyEntryGetCertificate); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } else if ((*env)->IsInstanceOf(env, loc[entry], g_TrustedCertificateEntryClass)) { flagsLocal |= EntryFlags_HasCertificate; loc[existingCert] = (*env)->CallObjectMethod(env, loc[entry], g_TrustedCertificateEntryGetTrustedCertificate); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } else { // Entry for alias exists, but doesn't represent a certificate or private key + certificate + ret = SUCCESS; goto cleanup; } assert(loc[existingCert] != NULL); - if ((*env)->CallBooleanMethod(env, cert, g_X509CertEquals, loc[existingCert])) + jboolean equals = (*env)->CallBooleanMethod(env, cert, g_X509CertEquals, loc[existingCert]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (equals) { flagsLocal |= EntryFlags_MatchesCertificate; } + ret = SUCCESS; + cleanup: RELEASE_LOCALS(loc, env); + *contains = containsLocal; *flags = flagsLocal; return ret; } +// Determines whether the store contains the specified alias with a matching certificate. +// Returns SUCCESS if the lookup completed (regardless of match result), FAIL on a JNI +// exception while inspecting the entry. On SUCCESS, *matches indicates whether the entry +// exists AND its certificate matches the supplied cert. ARGS_NON_NULL_ALL -static bool ContainsMatchingCertificateForAlias(JNIEnv* env, - jobject /*KeyStore*/ store, - jobject /*X509Certificate*/ cert, - jstring alias) +static int32_t ContainsMatchingCertificateForAlias(JNIEnv* env, + jobject /*KeyStore*/ store, + jobject /*X509Certificate*/ cert, + jstring alias, + bool* matches) { EntryFlags flags; - if (!ContainsEntryForAlias(env, store, cert, alias, &flags)) - return false; + bool contains = false; + *matches = false; + + if (ContainsEntryForAlias(env, store, cert, alias, &contains, &flags) != SUCCESS) + return FAIL; + + if (!contains) + return SUCCESS; EntryFlags matchesFlags = EntryFlags_HasCertificate & EntryFlags_MatchesCertificate; - return (flags & matchesFlags) == matchesFlags; + *matches = (flags & matchesFlags) == matchesFlags; + return SUCCESS; } int32_t AndroidCryptoNative_X509StoreAddCertificate(jobject /*KeyStore*/ store, @@ -98,7 +126,13 @@ int32_t AndroidCryptoNative_X509StoreAddCertificate(jobject /*KeyStore*/ store, jstring alias = make_java_string(env, hashString); EntryFlags flags; - if (ContainsEntryForAlias(env, store, cert, alias, &flags)) + bool contains = false; + if (ContainsEntryForAlias(env, store, cert, alias, &contains, &flags) != SUCCESS) + { + ReleaseLRef(env, alias); + return FAIL; + } + if (contains) { ReleaseLRef(env, alias); EntryFlags matchesFlags = EntryFlags_HasCertificate & EntryFlags_MatchesCertificate; @@ -136,11 +170,15 @@ int32_t AndroidCryptoNative_X509StoreAddCertificateWithPrivateKey(jobject /*KeyS INIT_LOCALS(loc, alias, certs); jobject privateKey = NULL; + bool releasePrivateKey = true; loc[alias] = make_java_string(env, hashString); EntryFlags flags; - if (ContainsEntryForAlias(env, store, cert, loc[alias], &flags)) + bool contains = false; + if (ContainsEntryForAlias(env, store, cert, loc[alias], &contains, &flags) != SUCCESS) + goto cleanup; + if (contains) { EntryFlags matchesFlags = EntryFlags_HasCertificate & EntryFlags_MatchesCertificate; if ((flags & matchesFlags) != matchesFlags) @@ -161,21 +199,23 @@ int32_t AndroidCryptoNative_X509StoreAddCertificateWithPrivateKey(jobject /*KeyS // Delete existing entry. We will replace the existing cert with the cert + private key. // store.deleteEntry(alias); (*env)->CallVoidMethod(env, store, g_KeyStoreDeleteEntry, loc[alias]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } - bool releasePrivateKey = true; switch (algorithm) { case PAL_EC: { EC_KEY* ec = (EC_KEY*)key; privateKey = (*env)->CallObjectMethod(env, ec->keyPair, g_keyPairGetPrivateMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); break; } case PAL_DSA: { // key is a KeyPair jobject privateKey = (*env)->CallObjectMethod(env, key, g_keyPairGetPrivateMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); break; } case PAL_RSA: @@ -205,7 +245,7 @@ int32_t AndroidCryptoNative_X509StoreAddCertificateWithPrivateKey(jobject /*KeyS RELEASE_LOCALS(loc, env); if (releasePrivateKey) { - (*env)->DeleteLocalRef(env, privateKey); + ReleaseLRef(env, privateKey); } return ret; @@ -222,7 +262,13 @@ bool AndroidCryptoNative_X509StoreContainsCertificate(jobject /*KeyStore*/ store JNIEnv* env = GetJNIEnv(); jstring alias = make_java_string(env, hashString); - bool containsCert = ContainsMatchingCertificateForAlias(env, store, cert, alias); + bool containsCert = false; + if (ContainsMatchingCertificateForAlias(env, store, cert, alias, &containsCert) != SUCCESS) + { + // Lookup failed (JNI exception). Treat as "not contained" so the exception + // doesn't leak back to managed code. + containsCert = false; + } (*env)->DeleteLocalRef(env, alias); return containsCert; } @@ -290,8 +336,11 @@ EnumerateCertificates(JNIEnv* env, jobject /*KeyStore*/ store, EnumCertificatesC // Public publicKey = cert.getPublicKey(); // PrivateKey privateKey = entry.getPrivateKey(); loc[cert] = (*env)->CallObjectMethod(env, loc[entry], g_PrivateKeyEntryGetCertificate); + ON_EXCEPTION_PRINT_AND_GOTO(loop_cleanup); loc[publicKey] = (*env)->CallObjectMethod(env, loc[cert], g_X509CertGetPublicKey); + ON_EXCEPTION_PRINT_AND_GOTO(loop_cleanup); loc[privateKey] = (*env)->CallObjectMethod(env, loc[entry], g_PrivateKeyEntryGetPrivateKey); + ON_EXCEPTION_PRINT_AND_GOTO(loop_cleanup); PAL_KeyAlgorithm keyAlgorithm = PAL_UnknownAlgorithm; void* keyHandle = HandleFromKeys(env, loc[publicKey], loc[privateKey], &keyAlgorithm); @@ -305,10 +354,13 @@ EnumerateCertificates(JNIEnv* env, jobject /*KeyStore*/ store, EnumCertificatesC { // Certificate cert = entry.getTrustedCertificate(); loc[cert] = (*env)->CallObjectMethod(env, loc[entry], g_TrustedCertificateEntryGetTrustedCertificate); + ON_EXCEPTION_PRINT_AND_GOTO(loop_cleanup); cb(AddGRef(env, loc[cert]), NULL /*privateKey*/, PAL_UnknownAlgorithm, context); } loop_cleanup: + // Per-entry exceptions have been logged and cleared via ON_EXCEPTION_PRINT_AND_GOTO. + // We silently skip the failed entry and continue, preserving the original behavior. RELEASE_LOCALS(loc, env); hasNext = (*env)->CallBooleanMethod(env, aliases, g_EnumerationHasMoreElements); @@ -340,6 +392,11 @@ static bool SystemAliasFilter(JNIEnv* env, jstring alias) size_t prefixLen = (sizeof(systemPrefix) / sizeof(*systemPrefix)) - 1; const char* aliasPtr = (*env)->GetStringUTFChars(env, alias, NULL); + if (aliasPtr == NULL) + { + TryClearJNIExceptions(env); + return false; + } bool isSystem = (strncmp(aliasPtr, systemPrefix, prefixLen) == 0); (*env)->ReleaseStringUTFChars(env, alias, aliasPtr); return isSystem; @@ -454,7 +511,10 @@ int32_t AndroidCryptoNative_X509StoreRemoveCertificate(jobject /*KeyStore*/ stor int32_t ret = FAIL; jstring alias = make_java_string(env, hashString); - if (!ContainsMatchingCertificateForAlias(env, store, cert, alias)) + bool containsCert = false; + if (ContainsMatchingCertificateForAlias(env, store, cert, alias, &containsCert) != SUCCESS) + goto cleanup; + if (!containsCert) { // Certificate is not in store - nothing to do ret = SUCCESS; From 1430ce48e25c8e8b9e96450bb22863cc9b4822e0 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 29 May 2026 13:46:55 +0200 Subject: [PATCH 02/14] [Android] Address review feedback: prefer cleanup labels and ReleaseLRef In response to review feedback on PR #1, refactor functions touched by the previous commit so that they: - Use ON_EXCEPTION_PRINT_AND_GOTO(cleanup) with a single cleanup label instead of bare `if (CheckJNIExceptions(env))` blocks where a function has resources to release on either the success or error path. - Replace raw DeleteLocalRef calls with the NULL-safe ReleaseLRef wrapper. This makes it safe to fall through to cleanup with partially initialized locals. - Unify duplicated success/error cleanup blocks into a single cleanup section driven by a `ret` (or `success`) sentinel. - Place the exception check immediately after the JNI call that may have thrown, not after subsequent unrelated work. While refactoring, two pre-existing local-ref / global-ref leaks were incidentally fixed: - pal_ecc_import_export.c GetECKeyParameters: the original `error` block zeroed the output pointers but did not ReleaseGRef the global refs allocated via ToGRef. The new cleanup releases them on failure. - pal_ecc_import_export.c EcKeyCreateByExplicitParameters: `ec` and `keyPairGenerator` were only released on error, leaking on success. They are now in the INIT_LOCALS array so they are released in both cases. Validated on the same Android arm64 emulator + physical device used for the previous commit; System.Security.Cryptography.Tests reports 10738 passed / 0 failed and System.Net.Security.Tests reports 4854 passed / 4 pre-existing failures (DelayedCertificate SkipTestException + ALPN AuthenticationException wrapping). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pal_cipher.c | 27 ++- .../pal_dsa.c | 45 +++-- .../pal_ecc_import_export.c | 173 +++++++----------- .../pal_ecdh.c | 71 ++++--- .../pal_eckey.c | 24 ++- .../pal_hmac.c | 40 ++-- 6 files changed, 163 insertions(+), 217 deletions(-) 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 8e1e58624a52e4..175bbb6893b6bf 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 @@ -138,6 +138,7 @@ int32_t AndroidCryptoNative_CipherSetTagLength(CipherCtx* ctx, int32_t tagLength ARGS_NON_NULL_ALL static int32_t ReinitializeCipher(CipherCtx* ctx) { JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; // SecretKeySpec keySpec = new SecretKeySpec(key.getEncoded(), "AES"); // IvParameterSpec ivSpec = new IvParameterSpec(IV); or GCMParameterSpec for GCM/CCM @@ -167,25 +168,21 @@ ARGS_NON_NULL_ALL static int32_t ReinitializeCipher(CipherCtx* ctx) ivPsObj = (*env)->NewObject(env, g_ivPsClass, g_ivPsCtor, ivBytes); } - (*env)->DeleteLocalRef(env, ivBytes); - - if (CheckJNIExceptions(env)) - { - (*env)->DeleteLocalRef(env, algName); - (*env)->DeleteLocalRef(env, sksObj); - (*env)->DeleteLocalRef(env, ivPsObj); - (*env)->DeleteLocalRef(env, keyBytes); - return FAIL; - } + ReleaseLRef(env, ivBytes); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } (*env)->CallVoidMethod(env, ctx->cipher, g_cipherInitMethod, ctx->encMode, sksObj, ivPsObj); - (*env)->DeleteLocalRef(env, algName); - (*env)->DeleteLocalRef(env, sksObj); - (*env)->DeleteLocalRef(env, ivPsObj); - (*env)->DeleteLocalRef(env, keyBytes); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - return CheckJNIExceptions(env) ? FAIL : SUCCESS; + ret = SUCCESS; + +cleanup: + ReleaseLRef(env, algName); + ReleaseLRef(env, sksObj); + ReleaseLRef(env, ivPsObj); + ReleaseLRef(env, keyBytes); + return ret; } int32_t AndroidCryptoNative_CipherSetKeyAndIV(CipherCtx* ctx, uint8_t* key, uint8_t* iv, int32_t enc) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c index 09325ad4b7925a..57a1efe5ddf57b 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c @@ -92,25 +92,24 @@ int32_t AndroidCryptoNative_DsaSizeP(jobject dsa) abort_if_invalid_pointer_argument (dsa); JNIEnv* env = GetJNIEnv(); + int32_t bytes = -1; INIT_LOCALS(loc, algName, keyFactory, publicKey, publicKeySpec, p); loc[algName] = make_java_string(env, "DSA"); loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[algName]); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[publicKey] = (*env)->CallObjectMethod(env, dsa, g_keyPairGetPublicMethod); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[publicKeySpec] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[publicKey], g_DSAPublicKeySpecClass); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[p] = (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetP); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - int32_t bytes = AndroidCryptoNative_GetBigNumBytes(loc[p]); - RELEASE_LOCALS_ENV(loc, ReleaseLRef); - return bytes; + bytes = AndroidCryptoNative_GetBigNumBytes(loc[p]); -error: +cleanup: RELEASE_LOCALS_ENV(loc, ReleaseLRef); - return -1; + return bytes; } int32_t AndroidCryptoNative_DsaSignatureFieldSize(jobject dsa) @@ -221,25 +220,26 @@ int32_t AndroidCryptoNative_GetDsaParameters( abort_if_invalid_pointer_argument (xLength); JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; INIT_LOCALS(loc, algName, keyFactory, publicKey, publicKeySpec, privateKey, privateKeySpec); loc[algName] = make_java_string(env, "DSA"); loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[algName]); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[publicKey] = (*env)->CallObjectMethod(env, dsa, g_keyPairGetPublicMethod); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); loc[publicKeySpec] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[publicKey], g_DSAPublicKeySpecClass); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *p = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetP)); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *q = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetQ)); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *g = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetG)); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *y = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetY)); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *pLength = AndroidCryptoNative_GetBigNumBytes(*p); *qLength = AndroidCryptoNative_GetBigNumBytes(*q); *gLength = AndroidCryptoNative_GetBigNumBytes(*g); @@ -248,22 +248,21 @@ int32_t AndroidCryptoNative_GetDsaParameters( *x = NULL; *xLength = 0; loc[privateKey] = (*env)->CallObjectMethod(env, dsa, g_keyPairGetPrivateMethod); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (loc[privateKey]) { loc[privateKeySpec] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[privateKey], g_DSAPrivateKeySpecClass); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *x = ToGRef(env, (*env)->CallObjectMethod(env, loc[privateKeySpec], g_DSAPrivateKeySpecGetX)); - ON_EXCEPTION_PRINT_AND_GOTO(error); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *xLength = AndroidCryptoNative_GetBigNumBytes(*x); } - RELEASE_LOCALS_ENV(loc, ReleaseLRef); - return SUCCESS; + ret = SUCCESS; -error: +cleanup: RELEASE_LOCALS_ENV(loc, ReleaseLRef); - return FAIL; + return ret; } int32_t AndroidCryptoNative_DsaKeyCreateByExplicitParameters( diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c index b9ea77d974472e..ef2095fba73d9a 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c @@ -25,100 +25,71 @@ int32_t AndroidCryptoNative_GetECKeyParameters(const EC_KEY* key, abort_if_invalid_pointer_argument (cbD); JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; - // Get the public key - jobject publicKey = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPublicMethod); - if (CheckJNIExceptions(env)) - goto error; - - jobject Q = (*env)->CallObjectMethod(env, publicKey, g_ECPublicKeyGetW); - - (*env)->DeleteLocalRef(env, publicKey); + INIT_LOCALS(loc, publicKey, Q, xBn, yBn, privateKey, dBn); - if (CheckJNIExceptions(env)) - goto error; + *qx = *qy = NULL; + *cbQx = *cbQy = 0; + *d = NULL; + *cbD = 0; - jobject xBn = (*env)->CallObjectMethod(env, Q, g_ECPointGetAffineX); - if (CheckJNIExceptions(env)) - { - (*env)->DeleteLocalRef(env, Q); - goto error; - } - jobject yBn = (*env)->CallObjectMethod(env, Q, g_ECPointGetAffineY); + // Get the public key + loc[publicKey] = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPublicMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - (*env)->DeleteLocalRef(env, Q); + loc[Q] = (*env)->CallObjectMethod(env, loc[publicKey], g_ECPublicKeyGetW); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (CheckJNIExceptions(env)) - { - if (xBn) (*env)->DeleteLocalRef(env, xBn); - if (yBn) (*env)->DeleteLocalRef(env, yBn); - goto error; - } + loc[xBn] = (*env)->CallObjectMethod(env, loc[Q], g_ECPointGetAffineX); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + loc[yBn] = (*env)->CallObjectMethod(env, loc[Q], g_ECPointGetAffineY); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - // Success; assign variables - *qx = ToGRef(env, xBn); + *qx = ToGRef(env, loc[xBn]); + loc[xBn] = NULL; *cbQx = AndroidCryptoNative_GetBigNumBytes(*qx); - xBn = NULL; - *qy = ToGRef(env, yBn); + *qy = ToGRef(env, loc[yBn]); + loc[yBn] = NULL; *cbQy = AndroidCryptoNative_GetBigNumBytes(*qy); - yBn = NULL; if (*cbQx == FAIL || *cbQy == FAIL) - goto error; + goto cleanup; if (includePrivate) { - abort_if_invalid_pointer_argument (d); - - jobject privateKey = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPrivateMethod); - if (CheckJNIExceptions(env)) - { - *d = NULL; - *cbD = 0; - goto error; - } + loc[privateKey] = (*env)->CallObjectMethod(env, key->keyPair, g_keyPairGetPrivateMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (!privateKey) - { - *d = NULL; - *cbD = 0; - goto error; - } + if (!loc[privateKey]) + goto cleanup; - jobject dBn = (*env)->CallObjectMethod(env, privateKey, g_ECPrivateKeyGetS); + loc[dBn] = (*env)->CallObjectMethod(env, loc[privateKey], g_ECPrivateKeyGetS); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - (*env)->DeleteLocalRef(env, privateKey); - - if (CheckJNIExceptions(env)) - { - *d = NULL; - *cbD = 0; - goto error; - } - - *d = ToGRef(env, dBn); + *d = ToGRef(env, loc[dBn]); + loc[dBn] = NULL; *cbD = AndroidCryptoNative_GetBigNumBytes(*d); if (*cbD == FAIL) - goto error; - } - else - { - if (d) - *d = NULL; - - if (cbD) - *cbD = 0; + goto cleanup; } - return SUCCESS; + ret = SUCCESS; -error: - *cbQx = *cbQy = 0; - *qx = *qy = 0; - if (d) +cleanup: + RELEASE_LOCALS_ENV(loc, ReleaseLRef); + if (ret != SUCCESS) + { + // Release any global refs we allocated via ToGRef and reset outputs + // so the caller sees a clean failure state. + ReleaseGRef(env, *qx); + ReleaseGRef(env, *qy); + ReleaseGRef(env, *d); + *qx = *qy = NULL; + *cbQx = *cbQy = 0; *d = NULL; - if (cbD) *cbD = 0; - return FAIL; + } + return ret; } int32_t AndroidCryptoNative_GetECCurveParameters(const EC_KEY* key, @@ -426,35 +397,34 @@ int32_t AndroidCryptoNative_EcKeyCreateByKeyParameters(EC_KEY** key, // Returns -1 if bigInteger < 0 or > INT32_MAX, or if a pending JNI exception is detected. ARGS_NON_NULL_ALL static int32_t ConvertBigIntegerToPositiveInt32(JNIEnv* env, jobject bigInteger) { + int32_t ret = -1; + jobject int32MaxBigInt = NULL; + jint signum = (*env)->CallIntMethod(env, bigInteger, g_sigNumMethod); - if (CheckJNIExceptions(env)) - return -1; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // bigInteger is negative. if (signum < 0) - { - return -1; - } + goto cleanup; - jobject int32MaxBigInt = (*env)->CallStaticObjectMethod(env, g_bigNumClass, g_valueOfMethod, (int64_t)INT32_MAX); - if (CheckJNIExceptions(env)) - return -1; + int32MaxBigInt = (*env)->CallStaticObjectMethod(env, g_bigNumClass, g_valueOfMethod, (int64_t)INT32_MAX); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); int willOverflow = (*env)->CallIntMethod(env, bigInteger, g_compareToMethod, int32MaxBigInt); - (*env)->DeleteLocalRef(env, int32MaxBigInt); - if (CheckJNIExceptions(env)) - return -1; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // If bigInteger > int32MaxBigInt, then a conversion to int32_t would be lossy. if (willOverflow > 0) - { - return -1; - } + goto cleanup; jint result = (*env)->CallIntMethod(env, bigInteger, g_intValueMethod); - if (CheckJNIExceptions(env)) - return -1; - return result; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + ret = result; + +cleanup: + ReleaseLRef(env, int32MaxBigInt); + return ret; } EC_KEY* AndroidCryptoNative_EcKeyCreateByExplicitParameters(ECCurveType curveType, @@ -496,7 +466,7 @@ EC_KEY* AndroidCryptoNative_EcKeyCreateByExplicitParameters(ECCurveType curveTyp EC_KEY* keyInfo = NULL; jobject keyPair = NULL; - INIT_LOCALS(loc, G, field, group, paramSpec, seedArray); + INIT_LOCALS(loc, G, field, group, paramSpec, seedArray, ec, keyPairGenerator); INIT_LOCALS(bn, P, A, B, GX, GY, ORDER, COFACTOR); // Java only supports Weierstrass and characteristic-2 type curves. @@ -588,23 +558,14 @@ EC_KEY* AndroidCryptoNative_EcKeyCreateByExplicitParameters(ECCurveType curveTyp else { // Otherwise generate a new key pair. - jstring ec = make_java_string(env, "EC"); - jobject keyPairGenerator = - (*env)->CallStaticObjectMethod(env, g_keyPairGenClass, g_keyPairGenGetInstanceMethod, ec); - if (CheckJNIExceptions(env)) - { - ReleaseLRef(env, ec); - goto error; - } - (*env)->CallVoidMethod(env, keyPairGenerator, g_keyPairGenInitializeWithParamsMethod, loc[paramSpec]); - if (CheckJNIExceptions(env)) - { - ReleaseLRef(env, keyPairGenerator); - ReleaseLRef(env, ec); - goto error; - } + loc[ec] = make_java_string(env, "EC"); + loc[keyPairGenerator] = + (*env)->CallStaticObjectMethod(env, g_keyPairGenClass, g_keyPairGenGetInstanceMethod, loc[ec]); + ON_EXCEPTION_PRINT_AND_GOTO(error); + (*env)->CallVoidMethod(env, loc[keyPairGenerator], g_keyPairGenInitializeWithParamsMethod, loc[paramSpec]); + ON_EXCEPTION_PRINT_AND_GOTO(error); - keyPair = AddGRef(env, (*env)->CallObjectMethod(env, keyPairGenerator, g_keyPairGenGenKeyPairMethod)); + keyPair = AddGRef(env, (*env)->CallObjectMethod(env, loc[keyPairGenerator], g_keyPairGenGenKeyPairMethod)); } if (!keyPair) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c index 0fbd35b1044057..a4981607b9d7ec 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c @@ -14,58 +14,51 @@ int32_t AndroidCryptoNative_EcdhDeriveKey(EC_KEY* ourKey, EC_KEY* peerKey, uint8 abort_if_invalid_pointer_argument (usedBufferLength); JNIEnv* env = GetJNIEnv(); + int32_t ret = FAIL; + *usedBufferLength = 0; - jstring algorithmName = make_java_string(env, "ECDH"); + jstring algorithmName = NULL; + jobject keyAgreement = NULL; + jobject privateKey = NULL; + jobject peerPublicKey = NULL; + jbyteArray secret = NULL; - jobject keyAgreement = (*env)->CallStaticObjectMethod(env, g_KeyAgreementClass, g_KeyAgreementGetInstance, algorithmName); - ReleaseLRef(env, algorithmName); - if (CheckJNIExceptions(env)) - { - *usedBufferLength = 0; - return FAIL; - } + algorithmName = make_java_string(env, "ECDH"); + + keyAgreement = (*env)->CallStaticObjectMethod(env, g_KeyAgreementClass, g_KeyAgreementGetInstance, algorithmName); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - jobject privateKey = (*env)->CallObjectMethod(env, ourKey->keyPair, g_keyPairGetPrivateMethod); + privateKey = (*env)->CallObjectMethod(env, ourKey->keyPair, g_keyPairGetPrivateMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, keyAgreement, g_KeyAgreementInit, privateKey); - ReleaseLRef(env, privateKey); - if (CheckJNIExceptions(env)) - { - ReleaseLRef(env, keyAgreement); - *usedBufferLength = 0; - return FAIL; - } + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - jobject peerPublicKey = (*env)->CallObjectMethod(env, peerKey->keyPair, g_keyPairGetPublicMethod); + peerPublicKey = (*env)->CallObjectMethod(env, peerKey->keyPair, g_keyPairGetPublicMethod); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ReleaseLRef(env, (*env)->CallObjectMethod(env, keyAgreement, g_KeyAgreementDoPhase, peerPublicKey, JNI_TRUE)); - ReleaseLRef(env, peerPublicKey); - if (CheckJNIExceptions(env)) - { - ReleaseLRef(env, keyAgreement); - *usedBufferLength = 0; - return FAIL; - } + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - jbyteArray secret = (*env)->CallObjectMethod(env, keyAgreement, g_KeyAgreementGenerateSecret); - ReleaseLRef(env, keyAgreement); - - if (CheckJNIExceptions(env)) - { - *usedBufferLength = 0; - return FAIL; - } + secret = (*env)->CallObjectMethod(env, keyAgreement, g_KeyAgreementGenerateSecret); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); jsize secretBufferLen = (*env)->GetArrayLength(env, secret); if (secretBufferLen > bufferLength) - { - ReleaseLRef(env, secret); - *usedBufferLength = 0; - return FAIL; - } + goto cleanup; (*env)->GetByteArrayRegion(env, secret, 0, secretBufferLen, (jbyte*)resultKey); - ReleaseLRef(env, secret); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *usedBufferLength = secretBufferLen; - return CheckJNIExceptions(env) ? FAIL : SUCCESS; + ret = SUCCESS; + +cleanup: + ReleaseLRef(env, secret); + ReleaseLRef(env, peerPublicKey); + ReleaseLRef(env, privateKey); + ReleaseLRef(env, keyAgreement); + ReleaseLRef(env, algorithmName); + if (ret != SUCCESS) + *usedBufferLength = 0; + return ret; } 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 a9bb7582016f48..0f5e704afdf4b1 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 @@ -153,21 +153,25 @@ int32_t AndroidCryptoNative_EcKeyGetSize(const EC_KEY* key, int32_t* keySize) return FAIL; JNIEnv* env = GetJNIEnv(); - - jobject curve = (*env)->CallObjectMethod(env, key->curveParameters, g_ECParameterSpecGetCurve); - jobject field = (*env)->CallObjectMethod(env, curve, g_EllipticCurveGetField); + int32_t ret = FAIL; + jobject curve = NULL; + jobject field = NULL; + + curve = (*env)->CallObjectMethod(env, key->curveParameters, g_ECParameterSpecGetCurve); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + field = (*env)->CallObjectMethod(env, curve, g_EllipticCurveGetField); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *keySize = (*env)->CallIntMethod(env, field, g_ECFieldGetFieldSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + ret = SUCCESS; +cleanup: ReleaseLRef(env, field); ReleaseLRef(env, curve); - - if (CheckJNIExceptions(env)) - { + if (ret != SUCCESS) *keySize = 0; - return FAIL; - } - - return SUCCESS; + return ret; } int32_t AndroidCryptoNative_EcKeyGetCurveName(const EC_KEY* key, uint16_t** curveName) 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 0f939dcfbce61f..0f9957bff8dea2 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 @@ -15,6 +15,8 @@ jobject CryptoNative_HmacCreate(uint8_t* key, int32_t keyLen, intptr_t type) // mac.init(key); JNIEnv* env = GetJNIEnv(); + jobject macObj = NULL; + bool success = false; jstring macName = NULL; if (type == CryptoNative_EvpSha1()) @@ -47,40 +49,30 @@ jobject CryptoNative_HmacCreate(uint8_t* key, int32_t keyLen, intptr_t type) } jobject sksObj = (*env)->NewObject(env, g_sksClass, g_sksCtor, keyBytes, macName); - if (CheckJNIExceptions(env) || sksObj == NULL) + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (sksObj == NULL) { - if(sksObj == NULL) - { - LOG_WARN ("Unable to create an instance of SecretKeySpec"); - } - - (*env)->DeleteLocalRef(env, keyBytes); - (*env)->DeleteLocalRef(env, sksObj); - (*env)->DeleteLocalRef(env, macName); - return FAIL; + LOG_WARN ("Unable to create an instance of SecretKeySpec"); + goto cleanup; } - jobject macObj = ToGRef(env, (*env)->CallStaticObjectMethod(env, g_MacClass, g_MacGetInstance, macName)); - if (CheckJNIExceptions(env)) - { - ReleaseGRef(env, macObj); - (*env)->DeleteLocalRef(env, keyBytes); - (*env)->DeleteLocalRef(env, sksObj); - (*env)->DeleteLocalRef(env, macName); - return FAIL; - } + macObj = ToGRef(env, (*env)->CallStaticObjectMethod(env, g_MacClass, g_MacGetInstance, macName)); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); (*env)->CallVoidMethod(env, macObj, g_MacInit, sksObj); - (*env)->DeleteLocalRef(env, keyBytes); - (*env)->DeleteLocalRef(env, sksObj); - (*env)->DeleteLocalRef(env, macName); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + success = true; - if (CheckJNIExceptions(env)) +cleanup: + ReleaseLRef(env, keyBytes); + ReleaseLRef(env, sksObj); + ReleaseLRef(env, macName); + if (!success) { ReleaseGRef(env, macObj); return FAIL; } - return macObj; } From 20239b14caf8fc4ef3a6d645457d49e729ddc477 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 29 May 2026 14:24:15 +0200 Subject: [PATCH 03/14] [Android] Avoid partial-output state on failure paths In response to the second round of review feedback, refactor the three functions that write to multiple jobject* out parameters so that they no longer leave the caller with a partially-populated output on failure: * pal_ecc_import_export.c GetECKeyParameters: keep all BigInteger results as local refs inside the INIT_LOCALS array and only promote them to global refs (via AddGRef) when the entire function succeeds. The cleanup path no longer needs to release global refs because none were created on the failure path. * pal_dsa.c GetDsaParameters: extend INIT_LOCALS with pBn, qBn, gBn, yBn, xBn; compute byte lengths into local int32_t variables; promote to *p/*q/*g/*y/*x only on success. * pal_cipher.c ReinitializeCipher: move ivBytes to function scope and put the exception check immediately after each NewObject call (not after ReleaseLRef). Cleanup releases ivBytes alongside the other locals so failure paths no longer leak it. Validated on the same Android arm64 emulator + physical device: System.Security.Cryptography.Tests 10738 passed / 0 failed System.Net.Security.Tests 4854 passed / 4 pre-existing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pal_cipher.c | 21 +++++---- .../pal_dsa.c | 45 +++++++++++++------ .../pal_ecc_import_export.c | 41 ++++++++--------- 3 files changed, 63 insertions(+), 44 deletions(-) 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 175bbb6893b6bf..4a2d5c67fd7599 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 @@ -140,23 +140,29 @@ ARGS_NON_NULL_ALL static int32_t ReinitializeCipher(CipherCtx* ctx) JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + jobject algName = NULL; + jbyteArray keyBytes = NULL; + jobject sksObj = NULL; + jbyteArray ivBytes = NULL; + jobject ivPsObj = NULL; + // SecretKeySpec keySpec = new SecretKeySpec(key.getEncoded(), "AES"); // IvParameterSpec ivSpec = new IvParameterSpec(IV); or GCMParameterSpec for GCM/CCM // cipher.init(encMode, keySpec, ivSpec); - jobject algName = GetAlgorithmName(env, ctx->type); + algName = GetAlgorithmName(env, ctx->type); if (!algName) - return FAIL; + goto cleanup; int32_t keyLength = ctx->keySizeInBits / 8; - jbyteArray keyBytes = make_java_byte_array(env, keyLength); + keyBytes = make_java_byte_array(env, keyLength); (*env)->SetByteArrayRegion(env, keyBytes, 0, keyLength, (jbyte*)ctx->key); - jobject sksObj = (*env)->NewObject(env, g_sksClass, g_sksCtor, keyBytes, algName); + sksObj = (*env)->NewObject(env, g_sksClass, g_sksCtor, keyBytes, algName); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - jobject ivPsObj = NULL; if (RequiresIV(ctx->type)) { - jbyteArray ivBytes = make_java_byte_array(env, ctx->ivLength); + ivBytes = make_java_byte_array(env, ctx->ivLength); (*env)->SetByteArrayRegion(env, ivBytes, 0, ctx->ivLength, (jbyte*)ctx->iv); if (HasVariableTag(ctx->type)) @@ -167,8 +173,6 @@ ARGS_NON_NULL_ALL static int32_t ReinitializeCipher(CipherCtx* ctx) { ivPsObj = (*env)->NewObject(env, g_ivPsClass, g_ivPsCtor, ivBytes); } - - ReleaseLRef(env, ivBytes); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } @@ -181,6 +185,7 @@ ARGS_NON_NULL_ALL static int32_t ReinitializeCipher(CipherCtx* ctx) ReleaseLRef(env, algName); ReleaseLRef(env, sksObj); ReleaseLRef(env, ivPsObj); + ReleaseLRef(env, ivBytes); ReleaseLRef(env, keyBytes); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c index 57a1efe5ddf57b..c586c3bf83d720 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c @@ -221,8 +221,12 @@ int32_t AndroidCryptoNative_GetDsaParameters( JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + int32_t cbP = 0, cbQ = 0, cbG = 0, cbY = 0, cbX = 0; - INIT_LOCALS(loc, algName, keyFactory, publicKey, publicKeySpec, privateKey, privateKeySpec); + INIT_LOCALS(loc, algName, keyFactory, publicKey, publicKeySpec, privateKey, privateKeySpec, pBn, qBn, gBn, yBn, xBn); + + *p = *q = *g = *y = *x = NULL; + *pLength = *qLength = *gLength = *yLength = *xLength = 0; loc[algName] = make_java_string(env, "DSA"); loc[keyFactory] = (*env)->CallStaticObjectMethod(env, g_KeyFactoryClass, g_KeyFactoryGetInstanceMethod, loc[algName]); @@ -232,30 +236,45 @@ int32_t AndroidCryptoNative_GetDsaParameters( loc[publicKeySpec] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[publicKey], g_DSAPublicKeySpecClass); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - *p = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetP)); + loc[pBn] = (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetP); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - *q = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetQ)); + loc[qBn] = (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetQ); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - *g = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetG)); + loc[gBn] = (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetG); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - *y = ToGRef(env, (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetY)); + loc[yBn] = (*env)->CallObjectMethod(env, loc[publicKeySpec], g_DSAPublicKeySpecGetY); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - *pLength = AndroidCryptoNative_GetBigNumBytes(*p); - *qLength = AndroidCryptoNative_GetBigNumBytes(*q); - *gLength = AndroidCryptoNative_GetBigNumBytes(*g); - *yLength = AndroidCryptoNative_GetBigNumBytes(*y); - *x = NULL; - *xLength = 0; + cbP = AndroidCryptoNative_GetBigNumBytes(loc[pBn]); + cbQ = AndroidCryptoNative_GetBigNumBytes(loc[qBn]); + cbG = AndroidCryptoNative_GetBigNumBytes(loc[gBn]); + cbY = AndroidCryptoNative_GetBigNumBytes(loc[yBn]); + loc[privateKey] = (*env)->CallObjectMethod(env, dsa, g_keyPairGetPrivateMethod); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (loc[privateKey]) { loc[privateKeySpec] = (*env)->CallObjectMethod(env, loc[keyFactory], g_KeyFactoryGetKeySpecMethod, loc[privateKey], g_DSAPrivateKeySpecClass); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - *x = ToGRef(env, (*env)->CallObjectMethod(env, loc[privateKeySpec], g_DSAPrivateKeySpecGetX)); + loc[xBn] = (*env)->CallObjectMethod(env, loc[privateKeySpec], g_DSAPrivateKeySpecGetX); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - *xLength = AndroidCryptoNative_GetBigNumBytes(*x); + cbX = AndroidCryptoNative_GetBigNumBytes(loc[xBn]); + } + + // Only promote local refs to global refs and write outputs on success so + // the caller never observes a partially-populated state. + *p = AddGRef(env, loc[pBn]); + *pLength = cbP; + *q = AddGRef(env, loc[qBn]); + *qLength = cbQ; + *g = AddGRef(env, loc[gBn]); + *gLength = cbG; + *y = AddGRef(env, loc[yBn]); + *yLength = cbY; + if (loc[xBn]) + { + *x = AddGRef(env, loc[xBn]); + *xLength = cbX; } ret = SUCCESS; diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c index ef2095fba73d9a..53de83ef2154ab 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c @@ -26,6 +26,7 @@ int32_t AndroidCryptoNative_GetECKeyParameters(const EC_KEY* key, JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + int32_t cbxBn = 0, cbyBn = 0, cbdBn = 0; INIT_LOCALS(loc, publicKey, Q, xBn, yBn, privateKey, dBn); @@ -46,13 +47,9 @@ int32_t AndroidCryptoNative_GetECKeyParameters(const EC_KEY* key, loc[yBn] = (*env)->CallObjectMethod(env, loc[Q], g_ECPointGetAffineY); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - *qx = ToGRef(env, loc[xBn]); - loc[xBn] = NULL; - *cbQx = AndroidCryptoNative_GetBigNumBytes(*qx); - *qy = ToGRef(env, loc[yBn]); - loc[yBn] = NULL; - *cbQy = AndroidCryptoNative_GetBigNumBytes(*qy); - if (*cbQx == FAIL || *cbQy == FAIL) + cbxBn = AndroidCryptoNative_GetBigNumBytes(loc[xBn]); + cbyBn = AndroidCryptoNative_GetBigNumBytes(loc[yBn]); + if (cbxBn == FAIL || cbyBn == FAIL) goto cleanup; if (includePrivate) @@ -66,29 +63,27 @@ int32_t AndroidCryptoNative_GetECKeyParameters(const EC_KEY* key, loc[dBn] = (*env)->CallObjectMethod(env, loc[privateKey], g_ECPrivateKeyGetS); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - *d = ToGRef(env, loc[dBn]); - loc[dBn] = NULL; - *cbD = AndroidCryptoNative_GetBigNumBytes(*d); - if (*cbD == FAIL) + cbdBn = AndroidCryptoNative_GetBigNumBytes(loc[dBn]); + if (cbdBn == FAIL) goto cleanup; } + // Only promote local refs to global refs and write outputs on success so + // the caller never observes a partially-populated state. + *qx = AddGRef(env, loc[xBn]); + *cbQx = cbxBn; + *qy = AddGRef(env, loc[yBn]); + *cbQy = cbyBn; + if (includePrivate) + { + *d = AddGRef(env, loc[dBn]); + *cbD = cbdBn; + } + ret = SUCCESS; cleanup: RELEASE_LOCALS_ENV(loc, ReleaseLRef); - if (ret != SUCCESS) - { - // Release any global refs we allocated via ToGRef and reset outputs - // so the caller sees a clean failure state. - ReleaseGRef(env, *qx); - ReleaseGRef(env, *qy); - ReleaseGRef(env, *d); - *qx = *qy = NULL; - *cbQx = *cbQy = 0; - *d = NULL; - *cbD = 0; - } return ret; } From bd14e7175a958871580c80ce2050ae9c271e60b9 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 29 May 2026 14:41:07 +0200 Subject: [PATCH 04/14] [Android] Apply no-partial-state pattern to remaining touched functions Generalize the previous round's principle (only write outputs on the success path) to the remaining functions modified by this PR: * pal_eckey.c EcKeyGetSize: use a local `size` variable for the CallIntMethod result and only assign `*keySize = size` once we reach the success path. The init-time `*keySize = 0` covers all failure paths so the cleanup-time re-zero is removed. * pal_x509store.c ContainsEntryForAlias: only copy `*contains` and `*flags` from the locals when `ret == SUCCESS`. On failure the caller already ignores them, but matching the documented behaviour avoids surprising future readers. * pal_ecdh.c EcdhDeriveKey: drop the cleanup-time `if (ret != SUCCESS) *usedBufferLength = 0;` since the function initializes `*usedBufferLength = 0` at entry and only writes the final value adjacent to `ret = SUCCESS`. Validated on the same Android arm64 emulator + physical device: System.Security.Cryptography.Tests 10738 passed / 0 failed System.Net.Security.Tests 4854 passed / 4 pre-existing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../System.Security.Cryptography.Native.Android/pal_ecdh.c | 2 -- .../pal_eckey.c | 6 +++--- .../pal_x509store.c | 7 +++++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c index a4981607b9d7ec..1b25b01adc073b 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c @@ -58,7 +58,5 @@ int32_t AndroidCryptoNative_EcdhDeriveKey(EC_KEY* ourKey, EC_KEY* peerKey, uint8 ReleaseLRef(env, privateKey); ReleaseLRef(env, keyAgreement); ReleaseLRef(env, algorithmName); - if (ret != SUCCESS) - *usedBufferLength = 0; return ret; } 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 0f5e704afdf4b1..d0002edc6edb52 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 @@ -154,6 +154,7 @@ int32_t AndroidCryptoNative_EcKeyGetSize(const EC_KEY* key, int32_t* keySize) JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; + int32_t size = 0; jobject curve = NULL; jobject field = NULL; @@ -161,16 +162,15 @@ int32_t AndroidCryptoNative_EcKeyGetSize(const EC_KEY* key, int32_t* keySize) ON_EXCEPTION_PRINT_AND_GOTO(cleanup); field = (*env)->CallObjectMethod(env, curve, g_EllipticCurveGetField); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - *keySize = (*env)->CallIntMethod(env, field, g_ECFieldGetFieldSize); + size = (*env)->CallIntMethod(env, field, g_ECFieldGetFieldSize); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + *keySize = size; ret = SUCCESS; cleanup: ReleaseLRef(env, field); ReleaseLRef(env, curve); - if (ret != SUCCESS) - *keySize = 0; return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509store.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509store.c index 981f34ff5a0e91..703546684d8d41 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509store.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509store.c @@ -83,8 +83,11 @@ ARGS_NON_NULL_ALL static int32_t ContainsEntryForAlias( cleanup: RELEASE_LOCALS(loc, env); - *contains = containsLocal; - *flags = flagsLocal; + if (ret == SUCCESS) + { + *contains = containsLocal; + *flags = flagsLocal; + } return ret; } From b86598b03813278e2f98462079e7ae4a08a1d738 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 29 May 2026 15:54:12 +0200 Subject: [PATCH 05/14] [Android] Use INIT_LOCALS for jobject locals and avoid partial-output state in SSLStreamInitialize Refactor ReinitializeCipher, EcdhDeriveKey and CryptoNative_HmacCreate to manage their jobject locals via INIT_LOCALS + RELEASE_LOCALS_ENV instead of ad-hoc stack locals released one-by-one in cleanup. This matches the convention used elsewhere in the file (e.g. SSLStreamSetTargetHost, GetQParameter, GetDsaParameters). Restore AndroidCryptoNative_SSLStreamInitialize to its original 'assign callback/handle fields at the end on the success path' pattern so that a failed Initialize does not leave the SSLStream in a partial state. The previous up-front assignment was unnecessary: FreeSSLStream now NULL-checks managedContextCleanup before invoking it, which is the correct safeguard for the partial-init case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pal_cipher.c | 32 +++++++---------- .../pal_ecdh.c | 30 ++++++---------- .../pal_hmac.c | 35 +++++++++---------- .../pal_sslstream.c | 12 +++---- 4 files changed, 43 insertions(+), 66 deletions(-) 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 4a2d5c67fd7599..93b6b0b49ac1e2 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 @@ -140,53 +140,45 @@ ARGS_NON_NULL_ALL static int32_t ReinitializeCipher(CipherCtx* ctx) JNIEnv* env = GetJNIEnv(); int32_t ret = FAIL; - jobject algName = NULL; - jbyteArray keyBytes = NULL; - jobject sksObj = NULL; - jbyteArray ivBytes = NULL; - jobject ivPsObj = NULL; + INIT_LOCALS(loc, algName, keyBytes, sksObj, ivBytes, ivPsObj); // SecretKeySpec keySpec = new SecretKeySpec(key.getEncoded(), "AES"); // IvParameterSpec ivSpec = new IvParameterSpec(IV); or GCMParameterSpec for GCM/CCM // cipher.init(encMode, keySpec, ivSpec); - algName = GetAlgorithmName(env, ctx->type); - if (!algName) + loc[algName] = GetAlgorithmName(env, ctx->type); + if (!loc[algName]) goto cleanup; int32_t keyLength = ctx->keySizeInBits / 8; - keyBytes = make_java_byte_array(env, keyLength); - (*env)->SetByteArrayRegion(env, keyBytes, 0, keyLength, (jbyte*)ctx->key); - sksObj = (*env)->NewObject(env, g_sksClass, g_sksCtor, keyBytes, algName); + loc[keyBytes] = make_java_byte_array(env, keyLength); + (*env)->SetByteArrayRegion(env, loc[keyBytes], 0, keyLength, (jbyte*)ctx->key); + loc[sksObj] = (*env)->NewObject(env, g_sksClass, g_sksCtor, loc[keyBytes], loc[algName]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (RequiresIV(ctx->type)) { - ivBytes = make_java_byte_array(env, ctx->ivLength); - (*env)->SetByteArrayRegion(env, ivBytes, 0, ctx->ivLength, (jbyte*)ctx->iv); + loc[ivBytes] = make_java_byte_array(env, ctx->ivLength); + (*env)->SetByteArrayRegion(env, loc[ivBytes], 0, ctx->ivLength, (jbyte*)ctx->iv); if (HasVariableTag(ctx->type)) { - ivPsObj = (*env)->NewObject(env, g_GCMParameterSpecClass, g_GCMParameterSpecCtor, ctx->tagLength * 8, ivBytes); + loc[ivPsObj] = (*env)->NewObject(env, g_GCMParameterSpecClass, g_GCMParameterSpecCtor, ctx->tagLength * 8, loc[ivBytes]); } else { - ivPsObj = (*env)->NewObject(env, g_ivPsClass, g_ivPsCtor, ivBytes); + loc[ivPsObj] = (*env)->NewObject(env, g_ivPsClass, g_ivPsCtor, loc[ivBytes]); } ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } - (*env)->CallVoidMethod(env, ctx->cipher, g_cipherInitMethod, ctx->encMode, sksObj, ivPsObj); + (*env)->CallVoidMethod(env, ctx->cipher, g_cipherInitMethod, ctx->encMode, loc[sksObj], loc[ivPsObj]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); ret = SUCCESS; cleanup: - ReleaseLRef(env, algName); - ReleaseLRef(env, sksObj); - ReleaseLRef(env, ivPsObj); - ReleaseLRef(env, ivBytes); - ReleaseLRef(env, keyBytes); + RELEASE_LOCALS_ENV(loc, ReleaseLRef); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c index 1b25b01adc073b..32dea29e58ae4f 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c @@ -17,46 +17,38 @@ int32_t AndroidCryptoNative_EcdhDeriveKey(EC_KEY* ourKey, EC_KEY* peerKey, uint8 int32_t ret = FAIL; *usedBufferLength = 0; - jstring algorithmName = NULL; - jobject keyAgreement = NULL; - jobject privateKey = NULL; - jobject peerPublicKey = NULL; - jbyteArray secret = NULL; + INIT_LOCALS(loc, algorithmName, keyAgreement, privateKey, peerPublicKey, secret); - algorithmName = make_java_string(env, "ECDH"); + loc[algorithmName] = make_java_string(env, "ECDH"); - keyAgreement = (*env)->CallStaticObjectMethod(env, g_KeyAgreementClass, g_KeyAgreementGetInstance, algorithmName); + loc[keyAgreement] = (*env)->CallStaticObjectMethod(env, g_KeyAgreementClass, g_KeyAgreementGetInstance, loc[algorithmName]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - privateKey = (*env)->CallObjectMethod(env, ourKey->keyPair, g_keyPairGetPrivateMethod); + loc[privateKey] = (*env)->CallObjectMethod(env, ourKey->keyPair, g_keyPairGetPrivateMethod); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - (*env)->CallVoidMethod(env, keyAgreement, g_KeyAgreementInit, privateKey); + (*env)->CallVoidMethod(env, loc[keyAgreement], g_KeyAgreementInit, loc[privateKey]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - peerPublicKey = (*env)->CallObjectMethod(env, peerKey->keyPair, g_keyPairGetPublicMethod); + loc[peerPublicKey] = (*env)->CallObjectMethod(env, peerKey->keyPair, g_keyPairGetPublicMethod); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - ReleaseLRef(env, (*env)->CallObjectMethod(env, keyAgreement, g_KeyAgreementDoPhase, peerPublicKey, JNI_TRUE)); + ReleaseLRef(env, (*env)->CallObjectMethod(env, loc[keyAgreement], g_KeyAgreementDoPhase, loc[peerPublicKey], JNI_TRUE)); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - secret = (*env)->CallObjectMethod(env, keyAgreement, g_KeyAgreementGenerateSecret); + loc[secret] = (*env)->CallObjectMethod(env, loc[keyAgreement], g_KeyAgreementGenerateSecret); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - jsize secretBufferLen = (*env)->GetArrayLength(env, secret); + jsize secretBufferLen = (*env)->GetArrayLength(env, loc[secret]); if (secretBufferLen > bufferLength) goto cleanup; - (*env)->GetByteArrayRegion(env, secret, 0, secretBufferLen, (jbyte*)resultKey); + (*env)->GetByteArrayRegion(env, loc[secret], 0, secretBufferLen, (jbyte*)resultKey); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *usedBufferLength = secretBufferLen; ret = SUCCESS; cleanup: - ReleaseLRef(env, secret); - ReleaseLRef(env, peerPublicKey); - ReleaseLRef(env, privateKey); - ReleaseLRef(env, keyAgreement); - ReleaseLRef(env, algorithmName); + RELEASE_LOCALS_ENV(loc, ReleaseLRef); 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 0f9957bff8dea2..b524270f104bf3 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 @@ -18,26 +18,25 @@ jobject CryptoNative_HmacCreate(uint8_t* key, int32_t keyLen, intptr_t type) jobject macObj = NULL; bool success = false; - jstring macName = NULL; + INIT_LOCALS(loc, macName, keyBytes, sksObj); + if (type == CryptoNative_EvpSha1()) - macName = make_java_string(env, "HmacSHA1"); + loc[macName] = make_java_string(env, "HmacSHA1"); else if (type == CryptoNative_EvpSha256()) - macName = make_java_string(env, "HmacSHA256"); + loc[macName] = make_java_string(env, "HmacSHA256"); else if (type == CryptoNative_EvpSha384()) - macName = make_java_string(env, "HmacSHA384"); + loc[macName] = make_java_string(env, "HmacSHA384"); else if (type == CryptoNative_EvpSha512()) - macName = make_java_string(env, "HmacSHA512"); + loc[macName] = make_java_string(env, "HmacSHA512"); else if (type == CryptoNative_EvpMd5()) - macName = make_java_string(env, "HmacMD5"); + loc[macName] = make_java_string(env, "HmacMD5"); else - return FAIL; - - jbyteArray keyBytes; + goto cleanup; if (key && keyLen > 0) { - keyBytes = make_java_byte_array(env, keyLen); - (*env)->SetByteArrayRegion(env, keyBytes, 0, keyLen, (jbyte*)key); + loc[keyBytes] = make_java_byte_array(env, keyLen); + (*env)->SetByteArrayRegion(env, loc[keyBytes], 0, keyLen, (jbyte*)key); } else { @@ -45,29 +44,27 @@ jobject CryptoNative_HmacCreate(uint8_t* key, int32_t keyLen, intptr_t type) // so instead create an empty 1-byte length byte array that's initialized to 0. // the HMAC algorithm pads keys with zeros until the key is block-length, // so this effectively creates the same key as if it were a zero byte-length key. - keyBytes = make_java_byte_array(env, 1); + loc[keyBytes] = make_java_byte_array(env, 1); } - jobject sksObj = (*env)->NewObject(env, g_sksClass, g_sksCtor, keyBytes, macName); + loc[sksObj] = (*env)->NewObject(env, g_sksClass, g_sksCtor, loc[keyBytes], loc[macName]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - if (sksObj == NULL) + if (loc[sksObj] == NULL) { LOG_WARN ("Unable to create an instance of SecretKeySpec"); goto cleanup; } - macObj = ToGRef(env, (*env)->CallStaticObjectMethod(env, g_MacClass, g_MacGetInstance, macName)); + macObj = ToGRef(env, (*env)->CallStaticObjectMethod(env, g_MacClass, g_MacGetInstance, loc[macName])); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - (*env)->CallVoidMethod(env, macObj, g_MacInit, sksObj); + (*env)->CallVoidMethod(env, macObj, g_MacInit, loc[sksObj]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); success = true; cleanup: - ReleaseLRef(env, keyBytes); - ReleaseLRef(env, sksObj); - ReleaseLRef(env, macName); + RELEASE_LOCALS_ENV(loc, ReleaseLRef); if (!success) { ReleaseGRef(env, macObj); 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 d1d7ef43651000..80e32c4234c560 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 @@ -614,14 +614,6 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( int32_t ret = FAIL; JNIEnv* env = GetJNIEnv(); - // Assign callback/handle fields up front so that FreeSSLStream() can safely run - // its managedContextCleanup() invocation if any of the JNI-call failure paths - // below jump to 'exit'. - sslStream->managedContextHandle = managedContextHandle; - sslStream->streamReader = streamReader; - sslStream->streamWriter = streamWriter; - sslStream->managedContextCleanup = managedContextCleanup; - jobject sslEngine = NULL; if (peerHost) { @@ -672,6 +664,10 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( ToGRef(env, (*env)->CallStaticObjectMethod(env, g_ByteBuffer, g_ByteBufferAllocate, packetBufferSize)); ON_EXCEPTION_PRINT_AND_GOTO(exit); + sslStream->managedContextHandle = managedContextHandle; + sslStream->streamReader = streamReader; + sslStream->streamWriter = streamWriter; + sslStream->managedContextCleanup = managedContextCleanup; ret = SUCCESS; exit: From 395fdb3d02303eafe05865b6d07f9f63077ab68a Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 29 May 2026 16:00:54 +0200 Subject: [PATCH 06/14] [Android] Use RELEASE_LOCALS(loc, env) for consistency with surrounding code RELEASE_LOCALS(loc, env) is the more common convention across the crypto PAL (35 uses vs 15 for RELEASE_LOCALS_ENV) and makes the env dependency visible at the call site. The two macros are functionally equivalent for releasing local refs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../System.Security.Cryptography.Native.Android/pal_cipher.c | 2 +- .../libs/System.Security.Cryptography.Native.Android/pal_ecdh.c | 2 +- .../libs/System.Security.Cryptography.Native.Android/pal_hmac.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 93b6b0b49ac1e2..b140e84b55e552 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 @@ -178,7 +178,7 @@ ARGS_NON_NULL_ALL static int32_t ReinitializeCipher(CipherCtx* ctx) ret = SUCCESS; cleanup: - RELEASE_LOCALS_ENV(loc, ReleaseLRef); + RELEASE_LOCALS(loc, env); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c index 32dea29e58ae4f..35951da3c1951f 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c @@ -49,6 +49,6 @@ int32_t AndroidCryptoNative_EcdhDeriveKey(EC_KEY* ourKey, EC_KEY* peerKey, uint8 ret = SUCCESS; cleanup: - RELEASE_LOCALS_ENV(loc, ReleaseLRef); + 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 b524270f104bf3..a769b79b6b4688 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 @@ -64,7 +64,7 @@ jobject CryptoNative_HmacCreate(uint8_t* key, int32_t keyLen, intptr_t type) success = true; cleanup: - RELEASE_LOCALS_ENV(loc, ReleaseLRef); + RELEASE_LOCALS(loc, env); if (!success) { ReleaseGRef(env, macObj); From 88556c3a70f44e09d9ad877e7983cb433ae2ddb1 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Fri, 29 May 2026 21:41:23 +0200 Subject: [PATCH 07/14] Fix DoUnwrap silently swallowing NewDirectByteBuffer failure ON_EXCEPTION_PRINT_AND_GOTO calls CheckJNIExceptions, which clears the pending exception. After a goto into the cleanup block, the subsequent CheckJNIExceptions(env) at the end of the block always returned false (the exception was already cleared), so DoUnwrap silently continued past a failed NewDirectByteBuffer with netInBuffer in an unchanged state and position == 0. The unwrap then proceeded against stale buffer state instead of returning SSLStreamStatus_Error. Restructure DoUnwrap to follow the convention used elsewhere in this file (e.g. SSLStreamRead): a single 'cleanup:' label at the end with 'return ret;', and all early exits going through 'goto cleanup' after setting 'ret'. This also adds the missing exception check after ByteBuffer.put(). Reported by Copilot review on PR #128777. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pal_sslstream.c | 59 +++++++++++-------- 1 file changed, 35 insertions(+), 24 deletions(-) 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 80e32c4234c560..12f1c7b317eb17 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 @@ -224,52 +224,54 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoWrap(JNIEnv* env, SSLStream* sslS ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* sslStream, int* handshakeStatus) { + PAL_SSLStreamStatus ret = SSLStreamStatus_Error; + uint8_t* tmpNative = NULL; + jobject tmp = NULL; + jobject result = NULL; + // if (netInBuffer.position() == 0) // { // int netInBufferLimit = netInBuffer.limit(); - // ByteBuffer tmp = ByteBuffer.allocateDirect(netInBufferLimit); - // int count = streamReader(tmp, 0, netInBufferLimit); + // byte[] tmpNative = new byte[netInBufferLimit]; + // // ... fill tmpNative from the stream + // ByteBuffer tmp = ByteBuffer.allocateDirect(count); // netInBuffer.put(tmp); // } int position = (*env)->CallIntMethod(env, sslStream->netInBuffer, g_ByteBufferPosition); - if (CheckJNIExceptions(env)) - return SSLStreamStatus_Error; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (position == 0) { + // int netInBufferLimit = netInBuffer.limit(); int netInBufferLimit = (*env)->CallIntMethod(env, sslStream->netInBuffer, g_ByteBufferLimit); - if (CheckJNIExceptions(env)) - return SSLStreamStatus_Error; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - uint8_t* tmpNative = (uint8_t*)xmalloc((size_t)netInBufferLimit); + tmpNative = (uint8_t*)xmalloc((size_t)netInBufferLimit); int count = netInBufferLimit; // todo assert streamReader != 0 ? PAL_SSLStreamStatus status = sslStream->streamReader(sslStream->managedContextHandle, tmpNative, &count); if (status != SSLStreamStatus_OK) { - free(tmpNative); - return status; + ret = status; + goto cleanup; } - jobject tmp = (*env)->NewDirectByteBuffer(env, tmpNative, count); + // ByteBuffer tmp = ByteBuffer.allocateDirect(count); + tmp = (*env)->NewDirectByteBuffer(env, tmpNative, count); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + // netInBuffer.put(tmp); IGNORE_RETURN( (*env)->CallObjectMethod(env, sslStream->netInBuffer, g_ByteBufferPutBuffer, tmp)); -cleanup: - free(tmpNative); - (*env)->DeleteLocalRef(env, tmp); - if (CheckJNIExceptions(env)) - return SSLStreamStatus_Error; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); } // netInBuffer.flip(); // SSLEngineResult result = sslEngine.unwrap(netInBuffer, appInBuffer); IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->netInBuffer, g_ByteBufferFlip)); - jobject result = (*env)->CallObjectMethod( + result = (*env)->CallObjectMethod( env, sslStream->sslEngine, g_SSLEngineUnwrap, sslStream->netInBuffer, sslStream->appInBuffer); - if (CheckJNIExceptions(env)) - return SSLStreamStatus_Error; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // netInBuffer.compact(); IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->netInBuffer, g_ByteBufferCompact)); @@ -278,17 +280,18 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss // SSLEngineResult.Status status = result.getStatus(); *handshakeStatus = GetEnumAsInt(env, (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetHandshakeStatus)); int status = GetEnumAsInt(env, (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetStatus)); - (*env)->DeleteLocalRef(env, result); switch (status) { case STATUS__OK: { - return SSLStreamStatus_OK; + ret = SSLStreamStatus_OK; + break; } case STATUS__CLOSED: { - return Close(env, sslStream); + ret = Close(env, sslStream); + break; } case STATUS__BUFFER_UNDERFLOW: { @@ -296,7 +299,8 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss // int newRemaining = sslSession.getPacketBufferSize(); int32_t newRemaining = (*env)->CallIntMethod(env, sslStream->sslSession, g_SSLSessionGetPacketBufferSize); sslStream->netInBuffer = EnsureRemaining(env, sslStream->netInBuffer, newRemaining); - return SSLStreamStatus_OK; + ret = SSLStreamStatus_OK; + break; } case STATUS__BUFFER_OVERFLOW: { @@ -306,14 +310,21 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss (*env)->CallIntMethod(env, sslStream->sslSession, g_SSLSessionGetApplicationBufferSize) + (*env)->CallIntMethod(env, sslStream->appInBuffer, g_ByteBufferRemaining); sslStream->appInBuffer = ExpandBuffer(env, sslStream->appInBuffer, newCapacity); - return SSLStreamStatus_OK; + ret = SSLStreamStatus_OK; + break; } default: { LOG_ERROR("Unknown SSLEngineResult status: %d", status); - return SSLStreamStatus_Error; + break; } } + +cleanup: + free(tmpNative); + ReleaseLRef(env, tmp); + ReleaseLRef(env, result); + return ret; } ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoHandshake(JNIEnv* env, SSLStream* sslStream) From e76a96e5bf0b46a2841278c9d813fcf3ad4b08e1 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 1 Jun 2026 12:07:31 +0200 Subject: [PATCH 08/14] [Android] Release SSLStream context on init failure Register the managed context cleanup callback before JNI setup work in SSLStreamInitialize so native cleanup owns the handle even if initialization fails partway through. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pal_sslstream.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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 12f1c7b317eb17..2fb4da05ae8184 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 @@ -366,10 +366,9 @@ ARGS_NON_NULL_ALL static void FreeSSLStream(JNIEnv* env, SSLStream* sslStream) ReleaseGRef(env, sslStream->netInBuffer); ReleaseGRef(env, sslStream->appInBuffer); - // managedContextCleanup may be NULL if the SSLStream was created via SSLStreamCreate - // but SSLStreamInitialize was never called (e.g. when managed code fails validation - // before invoking initialize and then disposes the SafeSslHandle). In that case there - // is no managed handle to release. + // managedContextCleanup may be NULL if the SSLStream was created but + // SSLStreamInitialize was never called. In that case there is no managed + // handle to release. if (sslStream->managedContextCleanup != NULL) sslStream->managedContextCleanup(sslStream->managedContextHandle); @@ -622,6 +621,11 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( abort_unless(sslStream->sslEngine == NULL, "sslEngine is NOT NULL in SSL stream"); abort_unless(sslStream->sslSession == NULL, "sslSession is NOT NULL in SSL stream"); + sslStream->managedContextHandle = managedContextHandle; + sslStream->streamReader = streamReader; + sslStream->streamWriter = streamWriter; + sslStream->managedContextCleanup = managedContextCleanup; + int32_t ret = FAIL; JNIEnv* env = GetJNIEnv(); @@ -675,10 +679,6 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( ToGRef(env, (*env)->CallStaticObjectMethod(env, g_ByteBuffer, g_ByteBufferAllocate, packetBufferSize)); ON_EXCEPTION_PRINT_AND_GOTO(exit); - sslStream->managedContextHandle = managedContextHandle; - sslStream->streamReader = streamReader; - sslStream->streamWriter = streamWriter; - sslStream->managedContextCleanup = managedContextCleanup; ret = SUCCESS; exit: From 3c30461bbde7b0435e7cd95d5ee31b84a179540e Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 1 Jun 2026 14:15:56 +0200 Subject: [PATCH 09/14] [Android] Handle JNI failures in crypto PAL review fixes Check and clean up JNI exceptions in remaining Android crypto PAL paths called out during review. Ensure global-ref promotion failures do not leave partially populated outputs, SSL protocol enumeration releases loop locals on error, and SSLStream initialization checks SSLEngine promotion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pal_dsa.c | 19 ++++++++++-- .../pal_ecc_import_export.c | 15 ++++++++-- .../pal_ecdh.c | 1 + .../pal_ssl.c | 30 +++++++++++++++++-- .../pal_sslstream.c | 1 + 5 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c index c586c3bf83d720..459a0f495d1905 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c @@ -261,25 +261,40 @@ int32_t AndroidCryptoNative_GetDsaParameters( cbX = AndroidCryptoNative_GetBigNumBytes(loc[xBn]); } - // Only promote local refs to global refs and write outputs on success so - // the caller never observes a partially-populated state. + // Clean up any partially promoted global refs if a later promotion fails. *p = AddGRef(env, loc[pBn]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *pLength = cbP; *q = AddGRef(env, loc[qBn]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *qLength = cbQ; *g = AddGRef(env, loc[gBn]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *gLength = cbG; *y = AddGRef(env, loc[yBn]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *yLength = cbY; if (loc[xBn]) { *x = AddGRef(env, loc[xBn]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *xLength = cbX; } ret = SUCCESS; cleanup: + if (ret != SUCCESS) + { + ReleaseGRef(env, *p); + ReleaseGRef(env, *q); + ReleaseGRef(env, *g); + ReleaseGRef(env, *y); + ReleaseGRef(env, *x); + *p = *q = *g = *y = *x = NULL; + *pLength = *qLength = *gLength = *yLength = *xLength = 0; + } + RELEASE_LOCALS_ENV(loc, ReleaseLRef); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c index 53de83ef2154ab..09fd6a505a280c 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecc_import_export.c @@ -68,21 +68,32 @@ int32_t AndroidCryptoNative_GetECKeyParameters(const EC_KEY* key, goto cleanup; } - // Only promote local refs to global refs and write outputs on success so - // the caller never observes a partially-populated state. + // Clean up any partially promoted global refs if a later promotion fails. *qx = AddGRef(env, loc[xBn]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *cbQx = cbxBn; *qy = AddGRef(env, loc[yBn]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *cbQy = cbyBn; if (includePrivate) { *d = AddGRef(env, loc[dBn]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); *cbD = cbdBn; } ret = SUCCESS; cleanup: + if (ret != SUCCESS) + { + ReleaseGRef(env, *qx); + ReleaseGRef(env, *qy); + ReleaseGRef(env, *d); + *qx = *qy = *d = NULL; + *cbQx = *cbQy = *cbD = 0; + } + RELEASE_LOCALS_ENV(loc, ReleaseLRef); return ret; } diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c index 35951da3c1951f..457160c941d185 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ecdh.c @@ -39,6 +39,7 @@ int32_t AndroidCryptoNative_EcdhDeriveKey(EC_KEY* ourKey, EC_KEY* peerKey, uint8 ON_EXCEPTION_PRINT_AND_GOTO(cleanup); jsize secretBufferLen = (*env)->GetArrayLength(env, loc[secret]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); if (secretBufferLen > bufferLength) goto cleanup; diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ssl.c index 80405457af09c6..0b581e9b4e2f49 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_ssl.c @@ -8,6 +8,8 @@ PAL_SslProtocol AndroidCryptoNative_SSLGetSupportedProtocols(void) JNIEnv* env = GetJNIEnv(); PAL_SslProtocol supported = 0; INIT_LOCALS(loc, context, params, protocols); + jstring protocol = NULL; + const char* protocolStr = NULL; // SSLContext context = SSLContext.getDefault(); // SSLParameters params = context.getDefaultSSLParameters(); @@ -23,10 +25,21 @@ PAL_SslProtocol AndroidCryptoNative_SSLGetSupportedProtocols(void) size_t tlsv1Len = (sizeof(tlsv1) / sizeof(*tlsv1)) - 1; jsize count = (*env)->GetArrayLength(env, loc[protocols]); + ON_EXCEPTION_PRINT_AND_GOTO(error); for (int32_t i = 0; i < count; i++) { - jstring protocol = (*env)->GetObjectArrayElement(env, loc[protocols], i); - const char* protocolStr = (*env)->GetStringUTFChars(env, protocol, NULL); + protocol = (*env)->GetObjectArrayElement(env, loc[protocols], i); + ON_EXCEPTION_PRINT_AND_GOTO(error); + if (protocol == NULL) + goto error; + + protocolStr = (*env)->GetStringUTFChars(env, protocol, NULL); + if (protocolStr == NULL) + { + ON_EXCEPTION_PRINT_AND_GOTO(error); + goto error; + } + if (strncmp(protocolStr, tlsv1, tlsv1Len) == 0) { if (strlen(protocolStr) == tlsv1Len) @@ -48,10 +61,21 @@ PAL_SslProtocol AndroidCryptoNative_SSLGetSupportedProtocols(void) } (*env)->ReleaseStringUTFChars(env, protocol, protocolStr); - (*env)->DeleteLocalRef(env, protocol); + protocolStr = NULL; + ReleaseLRef(env, protocol); + protocol = NULL; } + goto cleanup; + +error: + supported = 0; + cleanup: + if (protocolStr != NULL) + (*env)->ReleaseStringUTFChars(env, protocol, protocolStr); + + ReleaseLRef(env, protocol); RELEASE_LOCALS(loc, env); return supported; } 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 2fb4da05ae8184..84e85c65a0c749 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 @@ -647,6 +647,7 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( // sslEngine.setUseClientMode(!isServer); sslStream->sslEngine = ToGRef(env, sslEngine); + ON_EXCEPTION_PRINT_AND_GOTO(exit); (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineSetUseClientMode, !isServer); ON_EXCEPTION_PRINT_AND_GOTO(exit); From a9da52a9cc72c170ccf756d36bd752d3f5ba06b6 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 1 Jun 2026 14:17:44 +0200 Subject: [PATCH 10/14] [Android] Guard SSL engine promotion failure Add the explicit NULL guard requested in review after promoting the Java SSLEngine to a global reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pal_sslstream.c | 3 +++ 1 file changed, 3 insertions(+) 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 84e85c65a0c749..efc5d9b1380545 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 @@ -648,6 +648,9 @@ int32_t AndroidCryptoNative_SSLStreamInitialize( // sslEngine.setUseClientMode(!isServer); sslStream->sslEngine = ToGRef(env, sslEngine); ON_EXCEPTION_PRINT_AND_GOTO(exit); + if (sslStream->sslEngine == NULL) + goto exit; + (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineSetUseClientMode, !isServer); ON_EXCEPTION_PRINT_AND_GOTO(exit); From abe003847141f5b697a0443fd9126d3df35412a2 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Mon, 1 Jun 2026 15:34:01 +0200 Subject: [PATCH 11/14] [Android] Check remaining crypto PAL JNI failures Handle the remaining review findings in DSA parameter export and SSLStream buffer/result processing. Check BigInteger length failures, split JNI result-status calls, and avoid continuing with pending exceptions from buffer growth helpers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pal_dsa.c | 4 + .../pal_sslstream.c | 132 +++++++++++++++--- 2 files changed, 119 insertions(+), 17 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c b/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c index 459a0f495d1905..777dbbb3545c46 100644 --- a/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c +++ b/src/native/libs/System.Security.Cryptography.Native.Android/pal_dsa.c @@ -249,6 +249,8 @@ int32_t AndroidCryptoNative_GetDsaParameters( cbQ = AndroidCryptoNative_GetBigNumBytes(loc[qBn]); cbG = AndroidCryptoNative_GetBigNumBytes(loc[gBn]); cbY = AndroidCryptoNative_GetBigNumBytes(loc[yBn]); + if (cbP == FAIL || cbQ == FAIL || cbG == FAIL || cbY == FAIL) + goto cleanup; loc[privateKey] = (*env)->CallObjectMethod(env, dsa, g_keyPairGetPrivateMethod); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); @@ -259,6 +261,8 @@ int32_t AndroidCryptoNative_GetDsaParameters( loc[xBn] = (*env)->CallObjectMethod(env, loc[privateKeySpec], g_DSAPrivateKeySpecGetX); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); cbX = AndroidCryptoNative_GetBigNumBytes(loc[xBn]); + if (cbX == FAIL) + goto cleanup; } // Clean up any partially promoted global refs if a later promotion fails. 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 efc5d9b1380545..6e32e80254bb6b 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 @@ -128,9 +128,21 @@ ARGS_NON_NULL_ALL static jobject ExpandBuffer(JNIEnv* env, jobject oldBuffer, in // ByteBuffer newBuffer = ByteBuffer.allocate(newCapacity); // newBuffer.put(oldBuffer); IGNORE_RETURN((*env)->CallObjectMethod(env, oldBuffer, g_ByteBufferFlip)); + if (CheckJNIExceptions(env)) + return NULL; + jobject newBuffer = ToGRef(env, (*env)->CallStaticObjectMethod(env, g_ByteBuffer, g_ByteBufferAllocate, newCapacity)); + if (CheckJNIExceptions(env) || newBuffer == NULL) + return NULL; + IGNORE_RETURN((*env)->CallObjectMethod(env, newBuffer, g_ByteBufferPutBuffer, oldBuffer)); + if (CheckJNIExceptions(env)) + { + ReleaseGRef(env, newBuffer); + return NULL; + } + ReleaseGRef(env, oldBuffer); return newBuffer; } @@ -138,8 +150,17 @@ ARGS_NON_NULL_ALL static jobject ExpandBuffer(JNIEnv* env, jobject oldBuffer, in ARGS_NON_NULL_ALL static jobject EnsureRemaining(JNIEnv* env, jobject oldBuffer, int32_t newRemaining) { IGNORE_RETURN((*env)->CallObjectMethod(env, oldBuffer, g_ByteBufferCompact)); + if (CheckJNIExceptions(env)) + return NULL; + int32_t oldPosition = (*env)->CallIntMethod(env, oldBuffer, g_ByteBufferPosition); + if (CheckJNIExceptions(env)) + return NULL; + int32_t oldRemaining = (*env)->CallIntMethod(env, oldBuffer, g_ByteBufferRemaining); + if (CheckJNIExceptions(env)) + return NULL; + if (oldRemaining < newRemaining) { // After compacting the oldBuffer, the oldPosition is equal to the number of bytes in the buffer at the moment @@ -154,52 +175,91 @@ ARGS_NON_NULL_ALL static jobject EnsureRemaining(JNIEnv* env, jobject oldBuffer, ARGS_NON_NULL_ALL static PAL_SSLStreamStatus WrapAndProcessResult(JNIEnv* env, SSLStream* sslStream, int* handshakeStatus, int* bytesConsumed, bool* repeat) { + PAL_SSLStreamStatus ret = SSLStreamStatus_Error; + jobject resultHandshakeStatus = NULL; + jobject resultStatus = NULL; + // SSLEngineResult result = sslEngine.wrap(appOutBuffer, netOutBuffer); jobject result = (*env)->CallObjectMethod( env, sslStream->sslEngine, g_SSLEngineWrap, sslStream->appOutBuffer, sslStream->netOutBuffer); - if (CheckJNIExceptions(env)) - return SSLStreamStatus_Error; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (result == NULL) + goto cleanup; // handshakeStatus = result.getHandshakeStatus(); // bytesConsumed = result.bytesConsumed(); // SSLEngineResult.Status status = result.getStatus(); - *handshakeStatus = GetEnumAsInt(env, (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetHandshakeStatus)); + resultHandshakeStatus = (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetHandshakeStatus); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (resultHandshakeStatus == NULL) + goto cleanup; + + *handshakeStatus = GetEnumAsInt(env, resultHandshakeStatus); + resultHandshakeStatus = NULL; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + *bytesConsumed = (*env)->CallIntMethod(env, result, g_SSLEngineResultBytesConsumed); - int status = GetEnumAsInt(env, (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetStatus)); - (*env)->DeleteLocalRef(env, result); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + resultStatus = (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetStatus); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (resultStatus == NULL) + goto cleanup; + + int status = GetEnumAsInt(env, resultStatus); + resultStatus = NULL; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); switch (status) { case STATUS__OK: { - return Flush(env, sslStream); + ret = Flush(env, sslStream); + break; } case STATUS__CLOSED: { (void)Flush(env, sslStream); (*env)->CallVoidMethod(env, sslStream->sslEngine, g_SSLEngineCloseOutbound); - return SSLStreamStatus_Closed; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = SSLStreamStatus_Closed; + break; } case STATUS__BUFFER_OVERFLOW: { // Expand buffer and repeat the wrap int32_t packetBufferSize = (*env)->CallIntMethod(env, sslStream->sslSession, g_SSLSessionGetPacketBufferSize); - sslStream->netOutBuffer = ExpandBuffer(env, sslStream->netOutBuffer, packetBufferSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + jobject netOutBuffer = ExpandBuffer(env, sslStream->netOutBuffer, packetBufferSize); + if (netOutBuffer == NULL) + goto cleanup; + + sslStream->netOutBuffer = netOutBuffer; *repeat = true; - return SSLStreamStatus_OK; + ret = SSLStreamStatus_OK; + break; } default: { LOG_ERROR("Unknown SSLEngineResult status: %d", status); - return SSLStreamStatus_Error; + break; } } + +cleanup: + ReleaseLRef(env, resultHandshakeStatus); + ReleaseLRef(env, resultStatus); + ReleaseLRef(env, result); + return ret; } ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoWrap(JNIEnv* env, SSLStream* sslStream, int* handshakeStatus, int* bytesConsumed) { // appOutBuffer.flip(); IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appOutBuffer, g_ByteBufferFlip)); + if (CheckJNIExceptions(env)) + return SSLStreamStatus_Error; bool repeat = false; PAL_SSLStreamStatus status = WrapAndProcessResult(env, sslStream, handshakeStatus, bytesConsumed, &repeat); @@ -218,6 +278,8 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoWrap(JNIEnv* env, SSLStream* sslS // appOutBuffer.compact(); IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->appOutBuffer, g_ByteBufferCompact)); + if (CheckJNIExceptions(env)) + return SSLStreamStatus_Error; return status; } @@ -228,6 +290,8 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss uint8_t* tmpNative = NULL; jobject tmp = NULL; jobject result = NULL; + jobject resultHandshakeStatus = NULL; + jobject resultStatus = NULL; // if (netInBuffer.position() == 0) // { @@ -272,14 +336,32 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss result = (*env)->CallObjectMethod( env, sslStream->sslEngine, g_SSLEngineUnwrap, sslStream->netInBuffer, sslStream->appInBuffer); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (result == NULL) + goto cleanup; // netInBuffer.compact(); IGNORE_RETURN((*env)->CallObjectMethod(env, sslStream->netInBuffer, g_ByteBufferCompact)); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); // handshakeStatus = result.getHandshakeStatus(); // SSLEngineResult.Status status = result.getStatus(); - *handshakeStatus = GetEnumAsInt(env, (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetHandshakeStatus)); - int status = GetEnumAsInt(env, (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetStatus)); + resultHandshakeStatus = (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetHandshakeStatus); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (resultHandshakeStatus == NULL) + goto cleanup; + + *handshakeStatus = GetEnumAsInt(env, resultHandshakeStatus); + resultHandshakeStatus = NULL; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + resultStatus = (*env)->CallObjectMethod(env, result, g_SSLEngineResultGetStatus); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (resultStatus == NULL) + goto cleanup; + + int status = GetEnumAsInt(env, resultStatus); + resultStatus = NULL; + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); switch (status) { @@ -298,7 +380,13 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss // Expand buffer // int newRemaining = sslSession.getPacketBufferSize(); int32_t newRemaining = (*env)->CallIntMethod(env, sslStream->sslSession, g_SSLSessionGetPacketBufferSize); - sslStream->netInBuffer = EnsureRemaining(env, sslStream->netInBuffer, newRemaining); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + jobject netInBuffer = EnsureRemaining(env, sslStream->netInBuffer, newRemaining); + if (netInBuffer == NULL) + goto cleanup; + + sslStream->netInBuffer = netInBuffer; ret = SSLStreamStatus_OK; break; } @@ -306,10 +394,18 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss { // Expand buffer // int newCapacity = sslSession.getApplicationBufferSize() + appInBuffer.remaining(); - int32_t newCapacity = - (*env)->CallIntMethod(env, sslStream->sslSession, g_SSLSessionGetApplicationBufferSize) + - (*env)->CallIntMethod(env, sslStream->appInBuffer, g_ByteBufferRemaining); - sslStream->appInBuffer = ExpandBuffer(env, sslStream->appInBuffer, newCapacity); + int32_t applicationBufferSize = + (*env)->CallIntMethod(env, sslStream->sslSession, g_SSLSessionGetApplicationBufferSize); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + int32_t appInBufferRemaining = (*env)->CallIntMethod(env, sslStream->appInBuffer, g_ByteBufferRemaining); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + + jobject appInBuffer = ExpandBuffer(env, sslStream->appInBuffer, applicationBufferSize + appInBufferRemaining); + if (appInBuffer == NULL) + goto cleanup; + + sslStream->appInBuffer = appInBuffer; ret = SSLStreamStatus_OK; break; } @@ -323,6 +419,8 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss cleanup: free(tmpNative); ReleaseLRef(env, tmp); + ReleaseLRef(env, resultHandshakeStatus); + ReleaseLRef(env, resultStatus); ReleaseLRef(env, result); return ret; } From d3dfc206881e0c99ad2631d69e37b4849633a58d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Jun 2026 09:52:25 +0000 Subject: [PATCH 12/14] Split GetHandshakeStatus JNI call Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --- .../pal_sslstream.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 6e32e80254bb6b..c19c04a94e28a1 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 @@ -39,7 +39,11 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss ARGS_NON_NULL_ALL static int GetHandshakeStatus(JNIEnv* env, SSLStream* sslStream) { // int handshakeStatus = sslEngine.getHandshakeStatus().ordinal(); - int handshakeStatus = GetEnumAsInt(env, (*env)->CallObjectMethod(env, sslStream->sslEngine, g_SSLEngineGetHandshakeStatus)); + jobject status = (*env)->CallObjectMethod(env, sslStream->sslEngine, g_SSLEngineGetHandshakeStatus); + if (CheckJNIExceptions(env) || status == NULL) + return -1; + + int handshakeStatus = GetEnumAsInt(env, status); if (CheckJNIExceptions(env)) return -1; From ae407ce453b3eabf17260bebacffe45738808327 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:37:17 +0000 Subject: [PATCH 13/14] Release GetHandshakeStatus JNI local Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --- .../pal_sslstream.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) 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 c19c04a94e28a1..19916295dc55ad 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 @@ -38,16 +38,21 @@ ARGS_NON_NULL_ALL static PAL_SSLStreamStatus DoUnwrap(JNIEnv* env, SSLStream* ss ARGS_NON_NULL_ALL static int GetHandshakeStatus(JNIEnv* env, SSLStream* sslStream) { + int ret = -1; + INIT_LOCALS(loc, status); + // int handshakeStatus = sslEngine.getHandshakeStatus().ordinal(); - jobject status = (*env)->CallObjectMethod(env, sslStream->sslEngine, g_SSLEngineGetHandshakeStatus); - if (CheckJNIExceptions(env) || status == NULL) - return -1; + loc[status] = (*env)->CallObjectMethod(env, sslStream->sslEngine, g_SSLEngineGetHandshakeStatus); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + if (loc[status] == NULL) + goto cleanup; - int handshakeStatus = GetEnumAsInt(env, status); - if (CheckJNIExceptions(env)) - return -1; + ret = GetEnumAsInt(env, loc[status]); + ON_EXCEPTION_PRINT_AND_GOTO(cleanup); - return handshakeStatus; +cleanup: + RELEASE_LOCALS(loc, env); + return ret; } static bool IsHandshaking(int handshakeStatus) From 0c3cdb05fb1617f157ad86ea1d6aad78cea248d5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Jun 2026 12:52:31 +0000 Subject: [PATCH 14/14] Preserve GetHandshakeStatus failure ret Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --- .../pal_sslstream.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 19916295dc55ad..c90458d0bafb29 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 @@ -47,8 +47,9 @@ ARGS_NON_NULL_ALL static int GetHandshakeStatus(JNIEnv* env, SSLStream* sslStrea if (loc[status] == NULL) goto cleanup; - ret = GetEnumAsInt(env, loc[status]); + int handshakeStatus = GetEnumAsInt(env, loc[status]); ON_EXCEPTION_PRINT_AND_GOTO(cleanup); + ret = handshakeStatus; cleanup: RELEASE_LOCALS(loc, env);