JNI: improve UB protections in sqlite3_bind_blob/text/text16().

FossilOrigin-Name: 5c8383210a87d7f9d37a27053b5b1b6f41794fa8612826c68c1ca49c495cbd97

diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c
index 8009592b6b14bc0d7003cc8ce151b52e29762658..245ce4f9e992c0afe89cf3614c966370e5aeb549 100644 (file)
--- a/ext/jni/src/c/sqlite3-jni.c
+++ b/ext/jni/src/c/sqlite3-jni.c
@@ -841,13 +841,23 @@ static const char * s3jni__jstring_to_mutf8(JNIEnv * const env, jstring v ){
 #define s3jni_jstring_to_mutf8(ARG) s3jni__jstring_to_mutf8(env, (ARG))
 #define s3jni_mutf8_release(ARG,VAR) if( VAR ) (*env)->ReleaseStringUTFChars(env, ARG, VAR)
 
-static jbyte * s3jni__jbyteArray_bytes(JNIEnv * const env, jbyteArray jBA ){
+/*
+** If jBA is not NULL then its GetByteArrayElements() value is
+** returned. If jBA is not NULL and nBA is not NULL then *nBA is set
+** to the GetArrayLength() of jBA. If GetByteArrayElements() requires
+** an allocation and that allocation fails then this function either
+** fails fatally or returns 0, depending on build-time options.
+ */
+static jbyte * s3jni__jbyteArray_bytes2(JNIEnv * const env, jbyteArray jBA, jsize * nBA ){
   jbyte * const rv = jBA ? (*env)->GetByteArrayElements(env, jBA, NULL) : 0;
   s3jni_oom_check( jBA ? !!rv : 1 );
+  if( jBA && nBA ) *nBA = (*env)->GetArrayLength(env, jBA);
   return rv;
 }
 
-#define s3jni_jbyteArray_bytes(jByteArray) s3jni__jbyteArray_bytes(env, (jByteArray))
+#define s3jni_jbyteArray_bytes2(jByteArray,ptrToSz) \
+  s3jni__jbyteArray_bytes2(env, (jByteArray), (ptrToSz))
+#define s3jni_jbyteArray_bytes(jByteArray) s3jni__jbyteArray_bytes2(env, (jByteArray), 0)
 #define s3jni_jbyteArray_release(jByteArray,jBytes) \
   if( jBytes ) (*env)->ReleaseByteArrayElements(env, jByteArray, jBytes, JNI_ABORT)
 #define s3jni_jbyteArray_commit(jByteArray,jBytes) \
@@ -1010,7 +1020,7 @@ static jstring s3jni__utf8_to_jstring(JNIEnv * const env,
 static char * s3jni__jstring_to_utf8(JNIEnv * const env,
                                     jstring jstr, int *nLen){
   jbyteArray jba;
-  jsize nBa;
+  jsize nBA;
   char *rv;
 
   if( !jstr ) return 0;
@@ -1024,12 +1034,12 @@ static char * s3jni__jstring_to_utf8(JNIEnv * const env,
     if( nLen ) *nLen = 0;
     return 0;
   }
-  nBa = (*env)->GetArrayLength(env, jba);
-  if( nLen ) *nLen = (int)nBa;
-  rv = s3jni_malloc( nBa + 1 );
+  nBA = (*env)->GetArrayLength(env, jba);
+  if( nLen ) *nLen = (int)nBA;
+  rv = s3jni_malloc( nBA + 1 );
   if( rv ){
-    (*env)->GetByteArrayRegion(env, jba, 0, nBa, (jbyte*)rv);
-    rv[nBa] = 0;
+    (*env)->GetByteArrayRegion(env, jba, 0, nBA, (jbyte*)rv);
+    rv[nBA] = 0;
   }
   S3JniUnrefLocal(jba);
   return rv;
@@ -2359,9 +2369,13 @@ S3JniApi(sqlite3_backup_step(),jint,1backup_1step)(
 S3JniApi(sqlite3_bind_blob(),jint,1bind_1blob)(
   JniArgsEnvClass, jlong jpStmt, jint ndx, jbyteArray baData, jint nMax
 ){
-  jbyte * const pBuf = baData ? s3jni_jbyteArray_bytes(baData) : 0;
+  jsize nBA = 0;
+  jbyte * const pBuf = baData ? s3jni_jbyteArray_bytes2(baData, &nBA) : 0;
   int rc;
   if( pBuf ){
+    if( nMax>nBA ){
+      nMax = nBA;
+    }
     rc = sqlite3_bind_blob(S3JniLongPtr_sqlite3_stmt(jpStmt), (int)ndx,
                            pBuf, (int)nMax, SQLITE_TRANSIENT);
     s3jni_jbyteArray_release(baData, pBuf);
@@ -2448,25 +2462,49 @@ S3JniApi(sqlite3_bind_parameter_name(),jstring,1bind_1parameter_1name)(
   return z ? s3jni_utf8_to_jstring(z, -1) : 0;
 }
 
+/*
+** Impl of sqlite3_bind_text/text16().
+*/
+static int s3jni__bind_text(int is16, JNIEnv *env, jlong jpStmt, jint ndx,
+                            jbyteArray baData, jint nMax){
+  jsize nBA = 0;
+  jbyte * const pBuf =
+    baData ? s3jni_jbyteArray_bytes2(baData, &nBA) : 0;
+  int rc;
+  if( pBuf ){
+    if( nMax>nBA ){
+      nMax = nBA;
+    }
+    /* Note that we rely on the Java layer having assured that baData
+       is NUL-terminated if nMax is negative. In order to avoid UB for
+       such cases, we do not expose the byte-limit arguments in the
+       public API. */
+    rc = is16
+      ? sqlite3_bind_text16(S3JniLongPtr_sqlite3_stmt(jpStmt), (int)ndx,
+                            pBuf, (int)nMax, SQLITE_TRANSIENT)
+      : sqlite3_bind_text(S3JniLongPtr_sqlite3_stmt(jpStmt), (int)ndx,
+                          (const char *)pBuf,
+                          (int)nMax, SQLITE_TRANSIENT);
+  }else{
+    rc = baData
+      ? sqlite3_bind_null(S3JniLongPtr_sqlite3_stmt(jpStmt), (int)ndx)
+      : SQLITE_NOMEM;
+  }
+  s3jni_jbyteArray_release(baData, pBuf);
+  return (jint)rc;
+
+}
+
 S3JniApi(sqlite3_bind_text(),jint,1bind_1text)(
   JniArgsEnvClass, jlong jpStmt, jint ndx, jbyteArray baData, jint nMax
 ){
-  jbyte * const pBuf = baData ? s3jni_jbyteArray_bytes(baData) : 0;
-  int const rc = sqlite3_bind_text(S3JniLongPtr_sqlite3_stmt(jpStmt), (int)ndx,
-                                   (const char *)pBuf,
-                                   (int)nMax, SQLITE_TRANSIENT);
-  s3jni_jbyteArray_release(baData, pBuf);
-  return (jint)rc;
+  return s3jni__bind_text(0, env, jpStmt, ndx, baData, nMax);
 }
 
 S3JniApi(sqlite3_bind_text16(),jint,1bind_1text16)(
   JniArgsEnvClass, jlong jpStmt, jint ndx, jbyteArray baData, jint nMax
 ){
-  jbyte * const pBuf = baData ? s3jni_jbyteArray_bytes(baData) : 0;
-  int const rc = sqlite3_bind_text16(S3JniLongPtr_sqlite3_stmt(jpStmt), (int)ndx,
-                                     pBuf, (int)nMax, SQLITE_TRANSIENT);
-  s3jni_jbyteArray_release(baData, pBuf);
-  return (jint)rc;
+  return s3jni__bind_text(1, env, jpStmt, ndx, baData, nMax);
 }
 
 S3JniApi(sqlite3_bind_value(),jint,1bind_1value)(
@@ -2575,10 +2613,10 @@ S3JniApi(sqlite3_blob_write(),jint,1blob_1write)(
 ){
   sqlite3_blob * const b = S3JniLongPtr_sqlite3_blob(jpBlob);
   jbyte * const pBuf = b ? s3jni_jbyteArray_bytes(jBa) : 0;
-  const jsize nBa = pBuf ? (*env)->GetArrayLength(env, jBa) : 0;
+  const jsize nBA = pBuf ? (*env)->GetArrayLength(env, jBa) : 0;
   int rc = SQLITE_MISUSE;
   if(b && pBuf){
-    rc = sqlite3_blob_write( b, pBuf, (int)nBa, (int)iOffset );
+    rc = sqlite3_blob_write( b, pBuf, (int)nBA, (int)iOffset );
   }
   s3jni_jbyteArray_release(jBa, pBuf);
   return (jint)rc;
@@ -2994,12 +3032,12 @@ S3JniApi(sqlite3_complete(),int,1complete)(
   JniArgsEnvClass, jbyteArray jSql
 ){
   jbyte * const pBuf = s3jni_jbyteArray_bytes(jSql);
-  const jsize nBa = pBuf ? (*env)->GetArrayLength(env, jSql) : 0;
+  const jsize nBA = pBuf ? (*env)->GetArrayLength(env, jSql) : 0;
   int rc;
 
-  assert( (nBa>0 ? 0==pBuf[nBa-1] : (pBuf ? 0==*pBuf : 1))
+  assert( (nBA>0 ? 0==pBuf[nBA-1] : (pBuf ? 0==*pBuf : 1))
           && "Byte array is not NUL-terminated." );
-  rc = (pBuf && 0==pBuf[(nBa ? nBa-1 : 0)])
+  rc = (pBuf && 0==pBuf[(nBA ? nBA-1 : 0)])
     ? sqlite3_complete( (const char *)pBuf )
     : (jSql ? SQLITE_NOMEM : SQLITE_MISUSE);
   s3jni_jbyteArray_release(jSql, pBuf);
@@ -4159,9 +4197,9 @@ static void result_blob_text(int as64     /* true for text64/blob64() mode */,
     return;
   }else if( jBa ){
     jbyte * const pBuf = s3jni_jbyteArray_bytes(jBa);
-    jsize nBa = (*env)->GetArrayLength(env, jBa);
-    if( nMax>=0 && nBa>(jsize)nMax ){
-      nBa = (jsize)nMax;
+    jsize nBA = (*env)->GetArrayLength(env, jBa);
+    if( nMax>=0 && nBA>(jsize)nMax ){
+      nBA = (jsize)nMax;
       /**
          From the sqlite docs:
 
@@ -4181,15 +4219,15 @@ static void result_blob_text(int as64     /* true for text64/blob64() mode */,
     if( as64 ){ /* 64-bit... */
       static const jsize nLimit64 =
         SQLITE_MAX_ALLOCATION_SIZE/*only _kinda_ arbitrary*/;
-      if( nBa > nLimit64 ){
+      if( nBA > nLimit64 ){
         sqlite3_result_error_toobig(pCx);
       }else if( asBlob ){
-        sqlite3_result_blob64(pCx, pBuf, (sqlite3_uint64)nBa,
+        sqlite3_result_blob64(pCx, pBuf, (sqlite3_uint64)nBA,
                               SQLITE_TRANSIENT);
       }else{ /* text64... */
         if( encodingTypeIsValid(eTextRep) ){
           sqlite3_result_text64(pCx, (const char *)pBuf,
-                                (sqlite3_uint64)nBa,
+                                (sqlite3_uint64)nBA,
                                 SQLITE_TRANSIENT, eTextRep);
         }else{
           sqlite3_result_error_code(pCx, SQLITE_FORMAT);
@@ -4197,27 +4235,27 @@ static void result_blob_text(int as64     /* true for text64/blob64() mode */,
       }
     }else{ /* 32-bit... */
       static const jsize nLimit = SQLITE_MAX_ALLOCATION_SIZE;
-      if( nBa > nLimit ){
+      if( nBA > nLimit ){
         sqlite3_result_error_toobig(pCx);
       }else if( asBlob ){
-        sqlite3_result_blob(pCx, pBuf, (int)nBa,
+        sqlite3_result_blob(pCx, pBuf, (int)nBA,
                             SQLITE_TRANSIENT);
       }else{
         switch( eTextRep ){
           case SQLITE_UTF8:
-            sqlite3_result_text(pCx, (const char *)pBuf, (int)nBa,
+            sqlite3_result_text(pCx, (const char *)pBuf, (int)nBA,
                                 SQLITE_TRANSIENT);
             break;
           case SQLITE_UTF16:
-            sqlite3_result_text16(pCx, (const char *)pBuf, (int)nBa,
+            sqlite3_result_text16(pCx, (const char *)pBuf, (int)nBA,
                                   SQLITE_TRANSIENT);
             break;
           case SQLITE_UTF16LE:
-            sqlite3_result_text16le(pCx, (const char *)pBuf, (int)nBa,
+            sqlite3_result_text16le(pCx, (const char *)pBuf, (int)nBA,
                                     SQLITE_TRANSIENT);
             break;
           case SQLITE_UTF16BE:
-            sqlite3_result_text16be(pCx, (const char *)pBuf, (int)nBa,
+            sqlite3_result_text16be(pCx, (const char *)pBuf, (int)nBA,
                                     SQLITE_TRANSIENT);
             break;
         }

diff --git a/ext/jni/src/org/sqlite/jni/capi/CApi.java b/ext/jni/src/org/sqlite/jni/capi/CApi.java
index a72184474452477e16821111b03a46e026c4a081..302cdb760e94a03674fd2f948e3f0c8853b86a0d 100644 (file)
--- a/ext/jni/src/org/sqlite/jni/capi/CApi.java
+++ b/ext/jni/src/org/sqlite/jni/capi/CApi.java
@@ -215,9 +215,10 @@ public final class CApi {
   );
 
   /**
-     Results are undefined if data is not null and n<0 || n>=data.length.
+     If n is negative, SQLITE_MISUSE is returned. If n>data.length
+     then n is silently truncated to data.length.
   */
-  public static int sqlite3_bind_blob(
+  static int sqlite3_bind_blob(
     @NotNull sqlite3_stmt stmt, int ndx, @Nullable byte[] data, int n
   ){
     return sqlite3_bind_blob(stmt.getNativePointer(), ndx, data, n);
@@ -325,11 +326,12 @@ public final class CApi {
      SQLITE_TRANSIENT for the final C API parameter. The byte array is
      assumed to be in UTF-8 encoding.
 
-     <p>Results are undefined if data is not null and
-     maxBytes>=utf8.length. If maxBytes is negative then results are
-     undefined if data is not null and does not contain a NUL byte.
+     <p>If data is not null and maxBytes>utf8.length then maxBytes is
+     silently truncated to utf8.length. If maxBytes is negative then
+     results are undefined if data is not null and does not contain a
+     NUL byte.
   */
-  public static int sqlite3_bind_text(
+  static int sqlite3_bind_text(
     @NotNull sqlite3_stmt stmt, int ndx, @Nullable byte[] utf8, int maxBytes
   ){
     return sqlite3_bind_text(stmt.getNativePointer(), ndx, utf8, maxBytes);
@@ -354,7 +356,7 @@ public final class CApi {
   public static int sqlite3_bind_text(
     @NotNull sqlite3_stmt stmt, int ndx, @Nullable byte[] utf8
   ){
-    return (null == utf8)
+    return ( null==utf8 )
       ? sqlite3_bind_null(stmt.getNativePointer(), ndx)
       : sqlite3_bind_text(stmt.getNativePointer(), ndx, utf8, utf8.length);
   }
@@ -368,7 +370,7 @@ public final class CApi {
      signature but requires that its input be encoded in UTF-16 in
      platform byte order.
   */
-  public static int sqlite3_bind_text16(
+  static int sqlite3_bind_text16(
     @NotNull sqlite3_stmt stmt, int ndx, @Nullable byte[] data, int maxBytes
   ){
     return sqlite3_bind_text16(stmt.getNativePointer(), ndx, data, maxBytes);
@@ -982,7 +984,7 @@ public final class CApi {
      necessary, however, and overloads are provided which gloss over
      that.
 
-     <p>Results are undefined if maxBytes>=sqlUtf8.length.
+     <p>Results are undefined if maxBytes>sqlUtf8.length.
 
      <p>This routine is private because its maxBytes value is not
      strictly necessary in the Java interface, as sqlUtf8.length tells

diff --git a/manifest b/manifest
index 293cd130bcf1fe55a0f61fb1967465298bc946ac..16ecd5eb71ef8c3a0f9ea6046a825e80ca08edc5 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Add\smissing\sScalarFunction.java\sto\sJNI\sbuild.
-D 2023-10-22T11:11:54.330
+C JNI:\simprove\sUB\sprotections\sin\ssqlite3_bind_blob/text/text16().
+D 2023-10-22T12:33:05.772
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -239,7 +239,7 @@ F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a3
 F ext/jni/GNUmakefile 5c3ac326bf3853486ebe0d70819abc790cc65c412182ce4ebd5012b008d9b059
 F ext/jni/README.md ef9ac115e97704ea995d743b4a8334e23c659e5534c3b64065a5405256d5f2f4
 F ext/jni/jar-dist.make 030aaa4ae71dd86e4ec5e7c1e6cd86f9dfa47c4592c070d2e35157e42498e1fa
-F ext/jni/src/c/sqlite3-jni.c 8d32ca0598a11370a9e92a6d111f38934c225056b42b13512175acf6e37eed4c
+F ext/jni/src/c/sqlite3-jni.c 6f6df9657989e9ca2cfdcc2fe9a71c279de56d5c941adfd09a0f24256de35c8f
 F ext/jni/src/c/sqlite3-jni.h b4c413a0d0c734683da1049cfcf89e35ae2719759d0656ec0f8c57188f18cab8
 F ext/jni/src/org/sqlite/jni/annotation/NotNull.java a99341e88154e70447596b1af6a27c586317df41a7e0f246fd41370cd7b723b2
 F ext/jni/src/org/sqlite/jni/annotation/Nullable.java 0b1879852707f752512d4db9d7edd0d8db2f0c2612316ce1c832715e012ff6ba
@@ -249,7 +249,7 @@ F ext/jni/src/org/sqlite/jni/capi/AggregateFunction.java bc29e986c866c2ddbbb9f93
 F ext/jni/src/org/sqlite/jni/capi/AuthorizerCallback.java 7ed409d5449684616cc924534e22ff6b07d361f12ad904b69ecb10e0568a8013
 F ext/jni/src/org/sqlite/jni/capi/AutoExtensionCallback.java 74cc4998a73d6563542ecb90804a3c4f4e828cb4bd69e61226d1a51f4646e759
 F ext/jni/src/org/sqlite/jni/capi/BusyHandlerCallback.java 7b8e19810c42b0ad21a04b5d8c804b32ee5905d137148703f16a75b612c380ca
-F ext/jni/src/org/sqlite/jni/capi/CApi.java 5d754b4bb57852d006ad046b2860eb23ba406f800846460b26beee5172df4fc3
+F ext/jni/src/org/sqlite/jni/capi/CApi.java bccb442ca81cd4decb1adae99006a60b7a9f54e5153842e738c01104e97d1de0
 F ext/jni/src/org/sqlite/jni/capi/CallbackProxy.java 0bfd6e56e8265c2f05c9207665707285534d78f8466ef0e0430c65677f00943d
 F ext/jni/src/org/sqlite/jni/capi/CollationCallback.java e29bcfc540fdd343e2f5cca4d27235113f2886acb13380686756d5cabdfd065a
 F ext/jni/src/org/sqlite/jni/capi/CollationNeededCallback.java f81cf10b79c52f9b2e9247d523d29ae48863935f60420eae35f257c38c80ce95
@@ -2136,8 +2136,8 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 4a0e2c6e17eefb296b7e91a13305b2260d6eb869a37bc5e5b79edaf3c19c810a
-R 3fe1108f65f1caab77f9fbf8767ff3b7
+P b8258103fb433d9f5cfa15661b5edf4e60128fb4161d8a18e5cc3253e5aed72b
+R 10dd3eddc3858c0c90f96d7af2dd213f
 U stephan
-Z 479ae6fc35bc2a0cdd617f9441ccbd5d
+Z 39a3acd541e66480809465cadb3002fa
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index 571d755baa1e31ff4db1ad89ddea6ad8c485576d..bcfb4efa9576b161c8fd810f851a153d18c803ff 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-b8258103fb433d9f5cfa15661b5edf4e60128fb4161d8a18e5cc3253e5aed72b
\ No newline at end of file
+5c8383210a87d7f9d37a27053b5b1b6f41794fa8612826c68c1ca49c495cbd97
\ No newline at end of file