Feature request: JNI hardening

[h3ph4est7s]

The AOSP include the capability to enable JNI extended checking system wide including the following checks.

1. Arrays: attempting to allocate a negative-sized array.
2. Bad pointers: passing a bad jarray/jclass/jobject/jstring to a JNI call, or passing a NULL pointer to a JNI call with a non-nullable argument.
3. Class names: passing anything but the “java/lang/String” style of class name to a JNI call.
4. Critical calls: making a JNI call between a “critical” get and its corresponding release.
5. Direct ByteBuffers: passing bad arguments to NewDirectByteBuffer.
6. Exceptions: making a JNI call while there’s an exception pending.
7. JNIEnvs: using a JNIEnv from the wrong thread.
8. jfieldIDs: using a NULL jfieldID, or using a jfieldID to set a field to a value of the wrong type (trying to assign a StringBuilder to a String field, say), or using a jfieldID for a static field to set an instance field or vice versa, or using a jfieldID from one class with instances of another class.
9. jmethodIDs: using the wrong kind of jmethodID when making a Call*Method JNI call: incorrect return type, static/non-static mismatch, wrong type for ‘this’ (for non-static calls) or wrong class (for static calls).
10. References: using DeleteGlobalRef/DeleteLocalRef on the wrong kind of reference.
11. Release modes: passing a bad release mode to a release call (something other than 0, JNI_ABORT, or JNI_COMMIT).
12. Type safety: returning an incompatible type from your native method (returning a StringBuilder from a method declared to return a String, say).
13. UTF-8: passing an invalid Modified UTF-8 byte sequence to a JNI call.

Steps to enable:

• set the system property dalvik.vm.checkjni to true

Reference: https://developer.android.com/training/articles/perf-jni#extended-checking

Proposal: Add an option in security to enable or disable this system wide along with an option to enable or disable it for specific apps in App Info -> Exploit protection compatibility mode

[flawedworld]

This is in Android as a debugging feature, rather than a hardening feature. I don't think it can really be used as a hardening feature with that said.

[girlbossceo]

We could probably move some (not all) of these features out/away from the debugging property instead.

[flawedworld]

I'm not sure it actually offers any security benefits - irrespective of the setting, if there is an issue, the process will crash.

[thestinger]

Certain parts may be useful for security. Those parts would need to be considered individually.

[h3ph4est7s]

imho there is security value in some of those checks. The totality of those contain redundant checks and introduce additional overhead but still some can be more than useful. a new hybrid indirect function table in here https://github.com/GrapheneOS/platform_art/blob/13/runtime/jni/jni_env_ext.cc#L76 with a jvm option that utilize some of the functionality from here https://github.com/GrapheneOS/platform_art/blob/13/runtime/jni/check_jni.cc and a new property utilized in here https://github.com/GrapheneOS/platform_frameworks_base/blob/13/core/jni/AndroidRuntime.cpp#L750 will probably do the trick.

[girlbossceo]

We should evaluate what JNI checks from here are security relevant first. We don't want to add all of them.