From: stephan Date: Mon, 16 Oct 2023 08:10:11 +0000 (+0000) Subject: JNI: after calling a Java-side UDF, zero-out the pointer of the Java-side sqlite3_con... X-Git-Tag: version-3.44.0~107 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=19179722d6bf7bff82bcf9a72b2d0346346e46ff;p=thirdparty%2Fsqlite.git JNI: after calling a Java-side UDF, zero-out the pointer of the Java-side sqlite3_context and sqlite3_value array entries to avoid misbehavior if a client makes the mistake of holding a reference to one of those objects. FossilOrigin-Name: 9fc3104f76a83d600beb11d91feb97bcea8bc7f7cda8cd73e7a6b81fbba879df --- diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index 792f5d6f78..dd643c30bd 100644 --- a/ext/jni/src/c/sqlite3-jni.c +++ b/ext/jni/src/c/sqlite3-jni.c @@ -1835,7 +1835,7 @@ typedef struct { ** final 2 arguments. Returns 0 on success, SQLITE_NOMEM on allocation ** error. On error *jCx and *jArgv will be set to 0. The output ** objects are of type org.sqlite.jni.sqlite3_context and -** array-of-org.sqlite3.jni.sqlite3_value, respectively. +** array-of-org.sqlite.jni.sqlite3_value, respectively. */ static int udf_args(JNIEnv *env, sqlite3_context * const cx, @@ -1867,6 +1867,28 @@ error_oom: return SQLITE_NOMEM; } +/* +** Requires that jCx and jArgv are sqlite3_context +** resp. array-of-sqlite3_value values initialized by udf_args(). This +** function zeroes out the nativePointer member of jCx and each entry +** in jArgv. This is a safety-net precaution to avoid undefined +** behavior if a Java-side UDF holds a reference to one of its +** arguments. This MUST be called from any function which successfully +** calls udf_args(), after calling the corresponding UDF and checking +** its exception status. It MUST NOT be called in any other case. +*/ +static void udf_unargs(JNIEnv *env, jobject jCx, int argc, jobjectArray jArgv){ + int i = 0; + assert(jCx); + NativePointerHolder_set(S3JniNph(sqlite3_context), jCx, 0); + for( ; i < argc; ++i ){ + jobject jsv = (*env)->GetObjectArrayElement(env, jArgv, i); + assert(jsv); + NativePointerHolder_set(S3JniNph(sqlite3_value), jsv, 0); + } +} + + /* ** Must be called immediately after a Java-side UDF callback throws. ** If translateToErr is true then it sets the exception's message in @@ -1926,6 +1948,7 @@ static int udf_xFSI(sqlite3_context* const pCx, int argc, rc = udf_report_exception(env, 'F'==zFuncType[1]/*xFunc*/, pCx, s->zFuncName, zFuncType); } + udf_unargs(env, args.jcx, argc, args.jargv); } S3JniUnrefLocal(args.jcx); S3JniUnrefLocal(args.jargv); @@ -5168,6 +5191,7 @@ static void s3jni_fts5_extension_function(Fts5ExtensionApi const *pApi, S3JniIfThrew{ udf_report_exception(env, 1, pCx, pAux->zFuncName, "call"); } + udf_unargs(env, jpCx, argc, jArgv); S3JniUnrefLocal(jpFts); S3JniUnrefLocal(jpCx); S3JniUnrefLocal(jArgv); diff --git a/ext/jni/src/org/sqlite/jni/Tester1.java b/ext/jni/src/org/sqlite/jni/Tester1.java index 66489407fb..43ba085fab 100644 --- a/ext/jni/src/org/sqlite/jni/Tester1.java +++ b/ext/jni/src/org/sqlite/jni/Tester1.java @@ -694,6 +694,8 @@ public class Tester1 implements Runnable { // These ValueHolders are just to confirm that the func did what we want... final ValueHolder xDestroyCalled = new ValueHolder<>(false); final ValueHolder xFuncAccum = new ValueHolder<>(0); + final ValueHolder neverEverDoThisInClientCode = new ValueHolder<>(null); + final ValueHolder neverEverDoThisInClientCode2 = new ValueHolder<>(null); // Create an SQLFunction instance using one of its 3 subclasses: // Scalar, Aggregate, or Window: @@ -704,6 +706,15 @@ public class Tester1 implements Runnable { new ScalarFunction(){ public void xFunc(sqlite3_context cx, sqlite3_value[] args){ affirm(db == sqlite3_context_db_handle(cx)); + if( null==neverEverDoThisInClientCode.value ){ + neverEverDoThisInClientCode2.value = cx; + neverEverDoThisInClientCode.value = args + /* !!!NEVER!!! hold a reference to an sqlite3_value + object like this in client code! They are ONLY legal + for the duration of their single call. We do it here + ONLY to test that the defenses against clients doing + this are working. */; + } int result = 0; for( sqlite3_value v : args ) result += sqlite3_value_int(v); xFuncAccum.value += result;// just for post-run testing @@ -729,6 +740,13 @@ public class Tester1 implements Runnable { affirm(1 == n); affirm(6 == xFuncAccum.value); affirm( !xDestroyCalled.value ); + affirm( null!=neverEverDoThisInClientCode.value ); + affirm( null!=neverEverDoThisInClientCode2.value ); + affirm( 0