From 6f8f9fbe2530c3ff82a13711b3fe8fd0dcae5c7a Mon Sep 17 00:00:00 2001 From: dan Date: Tue, 27 Jan 2026 14:00:59 +0000 Subject: [PATCH] Fix trivial buffer overreads in the sessions module that could occur when parsing changeset blobs. FossilOrigin-Name: 661878a62870023f7f54e8c591a0823dc457cb89780ab40c1891fb3d5e8f095f --- ext/session/session4.test | 1 + ext/session/sqlite3session.c | 36 +++++++++++++++----- ext/session/test_session.c | 66 ++++++++++++++++++++++++------------ manifest | 18 +++++----- manifest.uuid | 2 +- 5 files changed, 83 insertions(+), 40 deletions(-) diff --git a/ext/session/session4.test b/ext/session/session4.test index 55cb76f153..5e44a8eb60 100644 --- 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 2775c78980..5468dff281 100644 --- 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 6ad5b37749..d3dcd9f2b9 100644 --- a/ext/session/test_session.c +++ b/ext/session/test_session.c @@ -7,6 +7,8 @@ #include #include "tclsqlite.h" +#include + #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 d346d1eaee..267be5e5ea 100644 --- 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 fa632973fb..3911f1e1d8 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -d1b8e7740bee13a8cf199c6477ee20a4f8bcbbd9ec4096bcdc05a996fadf0d56 +661878a62870023f7f54e8c591a0823dc457cb89780ab40c1891fb3d5e8f095f -- 2.47.3