From 535b1f287574d4a9e41ba7574254cbf742ab4dbe Mon Sep 17 00:00:00 2001 From: dan Date: Thu, 9 Apr 2026 05:33:19 +0000 Subject: [PATCH] Fix some buffer overreads that might occur in the session module when handling corrupt changesets. FossilOrigin-Name: 8fcf92e15d87487703afc1129f3a89a8d4d72cb30d30a1a9151a5596473069bd --- ext/session/sessionC.test | 49 +++++++++++++++++++++++++++++++++--- ext/session/sqlite3session.c | 15 ++++++++--- ext/session/test_session.c | 21 ++++++++++++++++ manifest | 21 +++++++++------- manifest.tags | 4 +-- manifest.uuid | 2 +- 6 files changed, 94 insertions(+), 18 deletions(-) diff --git a/ext/session/sessionC.test b/ext/session/sessionC.test index 57a05bd454..afe9276083 100644 --- a/ext/session/sessionC.test +++ b/ext/session/sessionC.test @@ -208,14 +208,57 @@ grp delete reset_db set C [binary format c* {0x54 0xda 0xda 0xda 0xda 0xda}] -do_execsql_test 4.0 { +do_execsql_test 5.0 { CREATE TABLE t1(a PRIMARY KEY, b, c, d); } -breakpoint -do_test 4.1 { +do_test 5.1 { list [catch { sqlite3changeset_apply db $C noop xFilter } msg] $msg } {1 SQLITE_CORRUPT} +#------------------------------------------------------------------------- +# + +foreach {tn type C2hex C3hex} { + 1 changeset 54020100743100170001000000000000000A030374656E000306656C6576656E + 54020100743100170001000000000000000A030374656E0003FFFF01656C6576656E + + 2 patchset 50020100743100170001000000000000000A0306656C6576656E + 50020100743100170001000000000000000A03FFFF06656C6576656E + + 3 changeset 54020100743100170001000000000000000A030374656E000306656C6576656E + 54020100743100170001000000000000000A030374656E0003FFFF +} { + reset_db + + do_execsql_test 6.$tn.0 { + CREATE TABLE t1(x INTEGER PRIMARY KEY, y TEXT); + INSERT INTO t1 VALUES(1, 'one'); + INSERT INTO t1 VALUES(2, 'two'); + INSERT INTO t1 VALUES(3, 'three'); + } {} + + do_test 6.$tn.1 { + set C1 [${type}_from_sql { + INSERT INTO t1 VALUES(10, 'ten'); + }] + set C2 [${type}_from_sql { + UPDATE t1 SET y='eleven' WHERE x=10; + }] + db one { SELECT quote($C2) } + } "X'$C2hex'" + + do_test 6.$tn.2 { + changeset_to_list [sqlite3changeset_concat $C1 $C2] + } {{INSERT t1 0 X. {} {i 10 t eleven}}} + + set C3 [db one {SELECT unhex($C3hex)}] + + do_test 6.$tn.3 { + list [catch { sqlite3changeset_concat $C1 $C3 } msg] $msg + } {1 SQLITE_CORRUPT} +} + + finish_test diff --git a/ext/session/sqlite3session.c b/ext/session/sqlite3session.c index 7350dbb919..cb5f4c1cc0 100644 --- a/ext/session/sqlite3session.c +++ b/ext/session/sqlite3session.c @@ -356,6 +356,7 @@ static int sessionVarintGet(const u8 *aBuf, int *piVal){ static int sessionVarintGetSafe(const u8 *aBuf, int nBuf, int *piVal){ u8 aCopy[9]; const u8 *aRead = aBuf; + memset(aCopy, 0, sizeof(aCopy)); if( nBufaData[pIn->iNext + nByte++]; if( eType==SQLITE_TEXT || eType==SQLITE_BLOB ){ int n; - nByte += sessionVarintGet(&pIn->aData[pIn->iNext+nByte], &n); + int nRem = pIn->nData - (pIn->iNext + nByte); + nByte += sessionVarintGetSafe(&pIn->aData[pIn->iNext+nByte], nRem, &n); nByte += n; rc = sessionInputBuffer(pIn, nByte); }else if( eType==SQLITE_INTEGER || eType==SQLITE_FLOAT ){ nByte += 8; } } + if( (pIn->iNext+nByte)>pIn->nData ){ + rc = SQLITE_CORRUPT_BKPT; + } } *pnByte = nByte; return rc; @@ -5717,6 +5722,10 @@ struct sqlite3_changegroup { ** This function is called to merge two changes to the same row together as ** part of an sqlite3changeset_concat() operation. A new change object is ** allocated and a pointer to it stored in *ppNew. +** +** Because they have been vetted by sqlite3changegroup_add() or similar, +** both the aRec[] change and the pExist change are safe to use without +** checking for buffer overflows. */ static int sessionChangeMerge( SessionTable *pTab, /* Table structure */ @@ -5857,7 +5866,7 @@ static int sessionChangeMerge( memcpy(aCsr, aRec, nRec); aCsr += nRec; }else{ - if( 0==sessionMergeUpdate(&aCsr, pTab, bPatchset, aExist, 0,aRec,0) ){ + if( 0==sessionMergeUpdate(&aCsr, pTab, bPatchset, aExist,0,aRec,0) ){ sqlite3_free(pNew); pNew = 0; } diff --git a/ext/session/test_session.c b/ext/session/test_session.c index 7282e3b4ee..1b09714225 100644 --- a/ext/session/test_session.c +++ b/ext/session/test_session.c @@ -1115,6 +1115,21 @@ static int SQLITE_TCLAPI test_sqlite3changeset_invert( return rc; } +/* +** Copy buffer aIn[] to a new nIn byte buffer obtained from malloc(). Use +** plain malloc() instead of any Tcl function because valgrind and asan are +** better at detecting small overflows in that case. Avoid sqlite3_malloc() +** here because that means dealing with injected OOM errors. +** +** The caller is responsible for eventually calling free() on the returned +** value. +*/ +static u8 *copyToMalloc(const u8 *aIn, int nIn){ + u8 *pRet = malloc(nIn); + memcpy(pRet, aIn, nIn); + return pRet; +} + /* ** sqlite3changeset_concat LEFT RIGHT */ @@ -1145,6 +1160,9 @@ static int SQLITE_TCLAPI test_sqlite3changeset_concat( sLeft.nStream = test_tcl_integer(interp, SESSION_STREAM_TCL_VAR); sRight.nStream = sLeft.nStream; + sLeft.aData = copyToMalloc(sLeft.aData, sLeft.nData); + sRight.aData = copyToMalloc(sRight.aData, sRight.nData); + if( sLeft.nStream>0 ){ rc = sqlite3changeset_concat_strm( testStreamInput, (void*)&sLeft, @@ -1157,6 +1175,9 @@ static int SQLITE_TCLAPI test_sqlite3changeset_concat( ); } + free(sLeft.aData); + free(sRight.aData); + if( rc!=SQLITE_OK ){ rc = test_session_error(interp, rc, 0); }else{ diff --git a/manifest b/manifest index 8346aba359..a637c4fa71 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\sa\sbuffer\soverflow\sbug\sin\sa\srecent\scheck-in,\sreported\sby\sunsolicted\nemail\sfrom\sOpenAI/Codex. -D 2026-04-08T17:00:33.995 +C Fix\ssome\sbuffer\soverreads\sthat\smight\soccur\sin\sthe\ssession\smodule\swhen\shandling\scorrupt\schangesets. +D 2026-04-09T05:33:19.706 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea @@ -540,7 +540,7 @@ F ext/session/session8.test 326f3273abf9d5d2d7d559eee8f5994c4ea74a5d935562454605 F ext/session/session9.test 0c4a8fbe7a5031f50855f020f3408e1f07fd7859f1daa1629eadcec3422072d6 F ext/session/sessionA.test 1feeab0b8e03527f08f2f1defb442da25480138f F ext/session/sessionB.test c4fb7f8a688787111606e123a555f18ee04f65bb9f2a4bb2aa71d55ce4e6d02c -F ext/session/sessionC.test dc06e2e8c48982fb0dea948c86bd0ef6766eb3894f0167749d13f30fd10af844 +F ext/session/sessionC.test 2bd42225efdf5f5b1a20f75b672665bcd4f67e2a6d7ddf7420fe7bf523ba41f8 F ext/session/sessionD.test 470ff917dc849e2eb78142ade63aaabd729d773833cff0ff01bca0eda68a21ce F ext/session/sessionE.test b2010949c9d7415306f64e3c2072ddabc4b8250c98478d3c0c4d064bce83111d F ext/session/sessionF.test d37ed800881e742c208df443537bf29aa49fd56eac520d0f0c6df3e6320f3401 @@ -571,9 +571,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 165a880952fdc1e6397cb05e7d337ff5174596078c1fb0abd4f9933141c565e3 +F ext/session/sqlite3session.c d5c91d5b07d2b8e860f2782ae23f7b44ce929280e00645418ee84a0fd14525b2 F ext/session/sqlite3session.h 063e7bf7be2fff874456f452a224b5b3013b25682d108933b0351c93a1279b9c -F ext/session/test_session.c 21b820d8b8007587fa32721e4cba104765f796345656fd277e94c58ecba9c34d +F ext/session/test_session.c 2a02a68b522e2f3d4a64b2a4733af54b0f3e500769aeccd5bcbdd440103db069 F ext/wasm/GNUmakefile 68c750f173106d9d63f12c1edf1256c6f4bad9894b155da5db64322f4912de4b F ext/wasm/README-dist.txt f01081a850ce38a56706af6b481e3a7878e24e42b314cfcd4b129f0f8427066a F ext/wasm/README.md 2e87804e12c98f1d194b7a06162a88441d33bb443efcfe00dc6565a780d2f259 @@ -2197,8 +2197,11 @@ F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee F tool/warnings.sh a554d13f6e5cf3760f041b87939e3d616ec6961859c3245e8ef701d1eafc2ca2 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f F tool/winmain.c 00c8fb88e365c9017db14c73d3c78af62194d9644feaf60e220ab0f411f3604c -P 025abd4cf409fb9938e116289f23dc5bcd6d14feb46066221e691b146ee9b354 -R 7600f47bef4b02dcc59e25059aaa900d -U drh -Z c818e025424851b4793e1d43a62d77e7 +P be891a137af15897691250324e4d3d9c96f0c5fb414bca27d0c3bfdd3012a8a2 +R 3c37bf09a4d96a6794a05c0475c7df24 +T *branch * session-fixes +T *sym-session-fixes * +T -sym-trunk * +U dan +Z f9a6a87e3d60b28afdcae7eb8d4d14d0 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.tags b/manifest.tags index bec971799f..de02e62efd 100644 --- a/manifest.tags +++ b/manifest.tags @@ -1,2 +1,2 @@ -branch trunk -tag trunk +branch session-fixes +tag session-fixes diff --git a/manifest.uuid b/manifest.uuid index 9731891d07..ec8a541806 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -be891a137af15897691250324e4d3d9c96f0c5fb414bca27d0c3bfdd3012a8a2 +8fcf92e15d87487703afc1129f3a89a8d4d72cb30d30a1a9151a5596473069bd -- 2.47.3