Fix trivial buffer overreads in the sessions module that could occur when parsing...

FossilOrigin-Name: 661878a62870023f7f54e8c591a0823dc457cb89780ab40c1891fb3d5e8f095f

diff --git a/ext/session/session4.test b/ext/session/session4.test
index 55cb76f153e18ab3c4a7eeb8069ecc55706061e6..5e44a8eb60cf201c5d7f299b4fc8f1d0df0279d7 100644 (file)
--- a/ext/session/session4.test
+++ b/ext/session/session4.test
@@ -135,6 +135,7 @@ foreach {tn blob} {
   54 540101743400120003001200010000000000000002120002400C000000000002120002400C00000000000050040100000074310017FF0050040100000074310017FF7F00000000000000050100000000000000030100000003001700010000666F7572
   55 540101743400120003001200010000000000000002120002400C00000000000050040100000074310017000100010080000001000000020003010100000300170100000003001700010000666F7572
   56 5487ffffff7f
+  57 54015052494e5446110017120004
 } {
   do_test 2.$tn {
     set changeset [binary decode hex $blob]

diff --git a/ext/session/sqlite3session.c b/ext/session/sqlite3session.c
index 2775c78980683e44928d3d02242a3897b8c8a9de..5468dff2817903f5bb3c9eedafe7712558747d06 100644 (file)
--- a/ext/session/sqlite3session.c
+++ b/ext/session/sqlite3session.c
@@ -349,6 +349,20 @@ static int sessionVarintGet(const u8 *aBuf, int *piVal){
   return getVarint32(aBuf, *piVal);
 }
 
+/*
+** Read a varint value from buffer aBuf[], size nBuf bytes, into *piVal.
+** Return the number of bytes read.
+*/
+static int sessionVarintGetSafe(const u8 *aBuf, int nBuf, int *piVal){
+  u8 aCopy[5];
+  const u8 *aRead = aBuf;
+  if( nBuf<5 ){
+    memcpy(aCopy, aBuf, nBuf);
+    aRead = aCopy;
+  }
+  return getVarint32(aRead, *piVal);
+}
+
 /* Load an unaligned and unsigned 32-bit integer */
 #define SESSION_UINT32(x) (((u32)(x)[0]<<24)|((x)[1]<<16)|((x)[2]<<8)|(x)[3])
 
@@ -3561,7 +3575,8 @@ static int sessionReadRecord(
       u8 *aVal = &pIn->aData[pIn->iNext];
       if( eType==SQLITE_TEXT || eType==SQLITE_BLOB ){
         int nByte;
-        pIn->iNext += sessionVarintGet(aVal, &nByte);
+        int nRem = pIn->nData - pIn->iNext;
+        pIn->iNext += sessionVarintGetSafe(aVal, nRem, &nByte);
         rc = sessionInputBuffer(pIn, nByte);
         if( rc==SQLITE_OK ){
           if( nByte<0 || nByte>pIn->nData-pIn->iNext ){
@@ -3614,7 +3629,8 @@ static int sessionChangesetBufferTblhdr(SessionInput *pIn, int *pnByte){
 
   rc = sessionInputBuffer(pIn, 9);
   if( rc==SQLITE_OK ){
-    nRead += sessionVarintGet(&pIn->aData[pIn->iNext + nRead], &nCol);
+    int nBuf = pIn->nData - pIn->iNext;
+    nRead += sessionVarintGetSafe(&pIn->aData[pIn->iNext], nBuf, &nCol);
     /* The hard upper limit for the number of columns in an SQLite
     ** database table is, according to sqliteLimit.h, 32676. So 
     ** consider any table-header that purports to have more than 65536 
@@ -3774,10 +3790,10 @@ static int sessionChangesetNextOne(
     memset(p->apValue, 0, sizeof(sqlite3_value*)*p->nCol*2);
   }
 
-  /* Make sure the buffer contains at least 10 bytes of input data, or all
-  ** remaining data if there are less than 10 bytes available. This is
-  ** sufficient either for the 'T' or 'P' byte and the varint that follows
-  ** it, or for the two single byte values otherwise. */
+  /* Make sure the buffer contains at least 2 bytes of input data, or all
+  ** remaining data if there are less than 2 bytes available. This is
+  ** sufficient either for the 'T' or 'P' byte that begins a new table,
+  ** or for the "op" and "bIndirect" single bytes otherwise. */
   p->rc = sessionInputBuffer(&p->in, 2);
   if( p->rc!=SQLITE_OK ) return p->rc;
 
@@ -3807,11 +3823,13 @@ static int sessionChangesetNextOne(
     return (p->rc = SQLITE_CORRUPT_BKPT);
   }
 
-  p->op = op;
-  p->bIndirect = p->in.aData[p->in.iNext++];
-  if( p->op!=SQLITE_UPDATE && p->op!=SQLITE_DELETE && p->op!=SQLITE_INSERT ){
+  if( (op!=SQLITE_UPDATE && op!=SQLITE_DELETE && op!=SQLITE_INSERT) 
+   || (p->in.iNext>=p->in.nData)
+  ){
     return (p->rc = SQLITE_CORRUPT_BKPT);
   }
+  p->op = op;
+  p->bIndirect = p->in.aData[p->in.iNext++];
 
   if( paRec ){ 
     int nVal;                     /* Number of values to buffer */

diff --git a/ext/session/test_session.c b/ext/session/test_session.c
index 6ad5b37749280c940d5c859be4186f63c54e66f0..d3dcd9f2b9333aad722012d6fa1c688db2da4328 100644 (file)
--- a/ext/session/test_session.c
+++ b/ext/session/test_session.c
@@ -7,6 +7,8 @@
 #include <string.h>
 #include "tclsqlite.h"
 
+#include <stdlib.h>
+
 #ifndef SQLITE_AMALGAMATION
   typedef unsigned char u8;
 #endif
@@ -856,6 +858,21 @@ static int testStreamInput(
   return SQLITE_OK;
 }
 
+/*
+** This works like Tcl_GetByteArrayFromObj(), except that it returns a buffer
+** allocated using malloc() that must be freed by the caller. This is done
+** because Tcl's buffers are often padded by a few bytes, which prevents
+** small overreads from being detected when tests are run under asan.
+*/
+static void *testGetByteArrayFromObj(Tcl_Obj *p, int *pnByte){
+  int nByte = 0;
+  void *aByte = Tcl_GetByteArrayFromObj(p, &nByte);
+  void *aCopy = malloc(nByte ? nByte : 1);
+  memcpy(aCopy, aByte, nByte);
+  *pnByte = nByte;
+  return aCopy;
+}
+
 
 static int SQLITE_TCLAPI testSqlite3changesetApply(
   int iVersion,
@@ -920,7 +937,7 @@ static int SQLITE_TCLAPI testSqlite3changesetApply(
     return TCL_ERROR;
   }
   db = *(sqlite3 **)info.objClientData;
-  pChangeset = (void *)Tcl_GetByteArrayFromObj(objv[2], &nChangeset);
+  pChangeset = (void *)testGetByteArrayFromObj(objv[2], &nChangeset);
   ctx.pConflictScript = objv[3];
   ctx.pFilterScript = objc==5 ? objv[4] : 0;
   ctx.interp = interp;
@@ -972,6 +989,7 @@ static int SQLITE_TCLAPI testSqlite3changesetApply(
     }
   }
 
+  free(pChangeset);
   if( rc!=SQLITE_OK ){
     return test_session_error(interp, rc, 0);
   }else{
@@ -1195,7 +1213,12 @@ static int SQLITE_TCLAPI test_sqlite3session_foreach(
   pCS = objv[2];
   pScript = objv[3];
 
-  pChangeset = (void *)Tcl_GetByteArrayFromObj(pCS, &nChangeset);
+  /* Take a copy of the changeset into an exact sized buffer allocated 
+  ** using malloc(). The Tcl buffer will be padded by a few bytes, which
+  ** prevents small overreads from being detected by ASAN when the tests
+  ** are run.  */
+  pChangeset = (void*)testGetByteArrayFromObj(pCS, &nChangeset);
+
   sStr.nStream = test_tcl_integer(interp, SESSION_STREAM_TCL_VAR);
   if( isInvert ){
     int f = SQLITE_CHANGESETSTART_INVERT;
@@ -1216,32 +1239,33 @@ static int SQLITE_TCLAPI test_sqlite3session_foreach(
       rc = sqlite3changeset_start_strm(&pIter, testStreamInput, (void*)&sStr);
     }
   }
-  if( rc!=SQLITE_OK ){
-    return test_session_error(interp, rc, 0);
-  }
 
-  while( SQLITE_ROW==sqlite3changeset_next(pIter) ){
-    Tcl_Obj *pVar = 0;            /* Tcl value to set $VARNAME to */
-    pVar = testIterData(pIter);
-    Tcl_ObjSetVar2(interp, pVarname, 0, pVar, 0);
-    rc = Tcl_EvalObjEx(interp, pScript, 0);
-    if( rc!=TCL_OK && rc!=TCL_CONTINUE ){
-      sqlite3changeset_finalize(pIter);
-      return rc==TCL_BREAK ? TCL_OK : rc;
+  if( rc==SQLITE_OK ){
+    while( SQLITE_ROW==sqlite3changeset_next(pIter) ){
+      Tcl_Obj *pVar = 0;            /* Tcl value to set $VARNAME to */
+      pVar = testIterData(pIter);
+      Tcl_ObjSetVar2(interp, pVarname, 0, pVar, 0);
+      rc = Tcl_EvalObjEx(interp, pScript, 0);
+      if( rc!=TCL_OK && rc!=TCL_CONTINUE ){
+        sqlite3changeset_finalize(pIter);
+        free(pChangeset);
+        return rc==TCL_BREAK ? TCL_OK : rc;
+      }
     }
-  }
 
-  if( isCheckNext ){
-    int rc2 = sqlite3changeset_next(pIter);
-    rc = sqlite3changeset_finalize(pIter);
-    assert( (rc2==SQLITE_DONE && rc==SQLITE_OK) || rc2==rc );
-  }else{
-    rc = sqlite3changeset_finalize(pIter);
+    if( isCheckNext ){
+      int rc2 = sqlite3changeset_next(pIter);
+      rc = sqlite3changeset_finalize(pIter);
+      assert( (rc2==SQLITE_DONE && rc==SQLITE_OK) || rc2==rc );
+    }else{
+      rc = sqlite3changeset_finalize(pIter);
+    }
   }
+
+  free(pChangeset);
   if( rc!=SQLITE_OK ){
     return test_session_error(interp, rc, 0);
   }
-
   return TCL_OK;
 }
 

diff --git a/manifest b/manifest
index d346d1eaeedaaacb9f41af48ed1f422274c2bb23..267be5e5eaf3849aa842bbf54f61b79b0a849673 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Additional\susage\snotes\sadded\sto\sthe\sheader\scomment\sof\sthe\stmstmpvfs.c\nsource\sfile.\s\sNo\scode\schanges.
-D 2026-01-27T12:50:52.534
+C Fix\strivial\sbuffer\soverreads\sin\sthe\ssessions\smodule\sthat\scould\soccur\swhen\sparsing\schangeset\sblobs.
+D 2026-01-27T14:00:59.953
 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
@@ -541,7 +541,7 @@ F ext/session/changesetfuzz1.test 15b629004e58d5ffcc852e6842a603775bb64b1ce51254
 F ext/session/session1.test 5d2502922d38a1579076863827342379a1609ca6bae78c40691a2be1ed1be2aa x
 F ext/session/session2.test ee83bb973b9ce17ccce4db931cdcdae65eb40bbb22089b2fe6aa4f6be3b9303f
 F ext/session/session3.test 2cc1629cfb880243aec1a7251145e07b78411d851b39b2aa1390704550db8e6a
-F ext/session/session4.test 823f6f018fcbb8dacf61e2960f8b3b848d492b094f8b495eae1d9407d9ab7219
+F ext/session/session4.test ad0ddaaddb9a99dac433d83fc6674aae2af072b8f57e63a6b3f2d76f1a66e98c
 F ext/session/session5.test 716bc6fafd625ce60dfa62ae128971628c1a1169
 F ext/session/session6.test 35279f2ec45448cd2e24a61688219dc6cf7871757716063acf4a8b5455e1e926
 F ext/session/session8.test 326f3273abf9d5d2d7d559eee8f5994c4ea74a5d935562454605e6607ee29904
@@ -578,9 +578,9 @@ F ext/session/sessionrowid.test 85187c2f1b38861a5844868126f69f9ec62223a03449a98a
 F ext/session/sessionsize.test 8fcf4685993c3dbaa46a24183940ab9f5aa9ed0d23e5fb63bfffbdb56134b795
 F ext/session/sessionstat1.test 5e718d5888c0c49bbb33a7a4f816366db85f59f6a4f97544a806421b85dc2dec
 F ext/session/sessionwor.test 6fd9a2256442cebde5b2284936ae9e0d54bde692d0f5fd009ecef8511f4cf3fc
-F ext/session/sqlite3session.c 4cb3921d93bf65c9c4f74c046a469d953f3070fb5a0c6f50bbbe365aa8be9f48
+F ext/session/sqlite3session.c 6ebd02be470f36d41c4bd78927f39d507b62051ba025eacaed9936c769902a07
 F ext/session/sqlite3session.h 7404723606074fcb2afdc6b72c206072cdb2b7d8ba097ca1559174a80bc26f7a
-F ext/session/test_session.c 8766b5973a6323934cb51248f621c3dc87ad2a98f023c3cc280d79e7d78d36fb
+F ext/session/test_session.c ac35aaf9c789f61d55dbf9342000c2c19420455b3c5971b708455e20b67df428
 F ext/wasm/GNUmakefile a2698072853b67c39e92ca19835c65fbaa8b8884078a99c4e54b72b9ede8306e
 F ext/wasm/README-dist.txt f01081a850ce38a56706af6b481e3a7878e24e42b314cfcd4b129f0f8427066a
 F ext/wasm/README.md 2e87804e12c98f1d194b7a06162a88441d33bb443efcfe00dc6565a780d2f259
@@ -2193,8 +2193,8 @@ F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee
 F tool/warnings.sh d924598cf2f55a4ecbc2aeb055c10bd5f48114793e7ba25f9585435da29e7e98
 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f
 F tool/winmain.c 00c8fb88e365c9017db14c73d3c78af62194d9644feaf60e220ab0f411f3604c
-P 3d37da3cb5943a61f528e3002c4c3ac3d41e871d742d8e603bffcc4bc5bd42fd
-R 8a32f6cdd6aca81d8601f27b004d8314
-U drh
-Z d0b77a5a906394fca0dd542c83c29e20
+P d1b8e7740bee13a8cf199c6477ee20a4f8bcbbd9ec4096bcdc05a996fadf0d56
+R bc760db4f6d40a1b040ea672b3ec64be
+U dan
+Z 5871de5b90660b7eb05ce757898385bf
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index fa632973fb69731a7d8dadfc4a9f641b3575667f..3911f1e1d8ac44bdc3439704b167ab9147a7f50d 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-d1b8e7740bee13a8cf199c6477ee20a4f8bcbbd9ec4096bcdc05a996fadf0d56
+661878a62870023f7f54e8c591a0823dc457cb89780ab40c1891fb3d5e8f095f