From 98fced62003f49f455f2b2bc2abbfee0935c912e Mon Sep 17 00:00:00 2001 From: dan Date: Sat, 24 Oct 2026 14:37:18 +0000 Subject: [PATCH] Change the order of fields in an internal structure to avoid the possibility of leaking heap address information when a corrupt database is queried. FossilOrigin-Name: 3f9a0ba4f4dffac7a4b9de30e66d94498a1433d96e327cfff43134b8856f4ba1 --- manifest | 14 ++++--- manifest.uuid | 2 +- src/pcache1.c | 62 ++++++++++++++++++++-------- test/pcacheleak.test | 96 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 24 deletions(-) create mode 100644 test/pcacheleak.test diff --git a/manifest b/manifest index 9221f7ee4d..91d3201063 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\sa\srace\scondition\swith\sa\swriter\sthat\scould\scause\sa\sconnection\swith\sa\sread-only\s*-shm\sfile\sto\sreturn\sa\sspurious\sSQLITE_READONLY_RECOVERY\serror. -D 2026-10-23T18:14:43.080 +C Change\sthe\sorder\sof\sfields\sin\san\sinternal\sstructure\sto\savoid\sthe\spossibility\sof\sleaking\sheap\saddress\sinformation\swhen\sa\scorrupt\sdatabase\sis\squeried. +D 2026-10-24T14:37:18.034 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea @@ -795,7 +795,7 @@ F src/pager.h 53cabc1fd881a9c078eab7db6c233bc0ed29d8938fc05585f0e75e80d905d308 F src/parse.y 606d70804b1cfd426a86b893ac361d0474cde6abcbc745fb1efd82b06f102c80 F src/pcache.c 588cc3c5ccaaadde689ed35ce5c5c891a1f7b1f4d1f56f6cf0143b74d8ee6484 F src/pcache.h 00c5797ddf546ee0fe2828d7f75d1afbf1b40ba054f362905326c5ccca15e60c -F src/pcache1.c 6bc38b44961f767985ae5022fb7887eda206040f57475812473929c702f6799d +F src/pcache1.c e5e548a11c4c417c781351413905141139cc1d8d8449a0e618d0a438a64da477 F src/pragma.c 78e1cf2aa4c260555a09c01e0e387c6ba14c85ff25b46e07b8d2c721ae7093a2 F src/prepare.c 8c4f4571668700664d0da6b6d2fe3e8d9d8808e3d1c643f14024e25d966df418 F src/printf.c 4bd228ed9e9346d558362be562757ee5eef6f9bf483b7f02751ca42cf95174d3 @@ -1584,6 +1584,7 @@ F test/pagesize.test 5769fc62d8c890a83a503f67d47508dfdc543305 F test/parser1.test 1901cf7157fbf7842eaee4a7dea65e096c606f01d42740a7991fa85eba421d45 F test/pcache.test c8acbedd3b6fd0f9a7ca887a83b11d24a007972b F test/pcache2.test 8a801d2b8e4b0ebb99701f026a67a9e84634c8aa24799a842c44003b93250da1 +F test/pcacheleak.test ccd1bf439cd472e2a756564d205451fd10d5961c1b7ca0a54d87d1a4c909c618 F test/pendingrace.test e99efc5ab3584da3dfc8cd6a0ec4e5a42214820574f5ea24ee93f1d84655f463 F test/percentile.test fd78896fa882fa4fbf693640097859721f3629926c2ccf804af5bcb7001fd35b F test/permutations.test c919e9d9b9d17faccd543128ac714efbba0606c7051bcb54f264006eecdb4627 @@ -2288,8 +2289,9 @@ F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee F tool/warnings.sh a554d13f6e5cf3760f041b87939e3d616ec6961859c3245e8ef701d1eafc2ca2 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f F tool/winmain.c 00c8fb88e365c9017db14c73d3c78af62194d9644feaf60e220ab0f411f3604c -P 30a301835b588f9d57fc04af3140612bef781bdc9b19aa83515d77a24ab06ee0 -R fd5d1e247283508f75b05db9c14f6283 +P e2e7c8eb02daea0876fb25abccd96c769f55fba3e995f0e7cf29aa992d7c61a6 +Q +a5261fd58070d4cb10c627e54491ed7588d96f47073b19612aa669f684e8e5b5 +R 719fb20b4cd51374e296c164f09790ba U dan -Z 2043ca754c859d05031715b7b6a9538a +Z 8fa7fd42bd607213f874d496008e0e67 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 2975941a2a..e38fe978b7 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -e2e7c8eb02daea0876fb25abccd96c769f55fba3e995f0e7cf29aa992d7c61a6 +3f9a0ba4f4dffac7a4b9de30e66d94498a1433d96e327cfff43134b8856f4ba1 diff --git a/src/pcache1.c b/src/pcache1.c index 6f61e77beb..0f791d4648 100644 --- a/src/pcache1.c +++ b/src/pcache1.c @@ -99,6 +99,18 @@ typedef struct PGroup PGroup; ** object is positioned immediately after the page buffer to serve as an ** overrun area, so that overreads are harmless. ** +** Under some circumstances, using a strategically corrupt db file, one +** or two varints may be read from past the end of the page buffer and +** returned to the user (as rowid values). For this reason, we ensure that +** the first few elements of PgHdr1 are not pointers. If they were, then +** a determined attacker might be able to read the value of the pointer, +** which can be used as part of a larger exploit. Furthermore, because the +** first two elements of PgHdr1 are isBulkLocal and isAnchor - u16 variables +** that always contain either 1 or 0 - the PgHdr1 structure always begins +** with 4 bytes with the high-bit cleared, and so code attempting to read +** varints can never progress past this point and read any more sensitive +** information further inside the structure. +** ** Variables isBulkLocal and isAnchor were once type "u8". That works, ** but causes a 2-byte gap in the structure for most architectures (since ** pointers must be either 4 or 8-byte aligned). As this structure is located @@ -107,7 +119,10 @@ typedef struct PGroup PGroup; ** read part of this structure before the corruption is detected. This ** can cause a valgrind error if the uninitialized gap is accessed. Using u16 ** ensures there is no such gap, and therefore no bytes of uninitialized -** memory in the structure. +** memory in the structure. Keeping the u16 type also lets isBulkLocal, +** isAnchor and iKey fill exactly the 8 bytes (2+2+4) that precede the +** 8-byte-aligned "page" member, so no uninitialized padding is exposed to +** an overread. ** ** The pLruNext and pLruPrev pointers form a double-linked circular list ** of all pages that are unpinned. The PGroup.lru element (which should be @@ -115,10 +130,11 @@ typedef struct PGroup PGroup; ** beginning and the end of the list. */ struct PgHdr1 { - sqlite3_pcache_page page; /* Base class. Must be first. pBuf & pExtra */ - unsigned int iKey; /* Key value (page number) */ u16 isBulkLocal; /* This page from bulk local storage */ u16 isAnchor; /* This is the PGroup.lru element */ + unsigned int iKey; /* Key value (page number) */ + sqlite3_pcache_page page; /* Base class. pBuf & pExtra. Deliberately not the + ** first member - see the note above. */ PgHdr1 *pNext; /* Next in hash table chain */ PCache1 *pCache; /* Cache that currently owns this page */ PgHdr1 *pLruNext; /* Next in circular LRU list of unpinned pages */ @@ -126,6 +142,18 @@ struct PgHdr1 { /* NB: pLruPrev is only valid if pLruNext!=0 */ }; +/* +** Given the sqlite3_pcache_page object handed out to the pcache.c layer, +** recover the PgHdr1 that contains it. PgHdr1.page is no longer required to +** be the first member of PgHdr1, so this goes through the "page" member +** rather than casting the structure pointer. The fetch methods produce the +** other direction directly with &pPage->page, only on paths where pPage is +** known to be non-NULL. PAGE_TO_PGHDR1() is likewise only ever given a +** non-NULL page that this module previously handed out. +*/ +#define PAGE_TO_PGHDR1(pPg) \ + ((PgHdr1*)(((char*)(pPg)) - offsetof(PgHdr1,page))) + /* ** A page is pinned if it is not on the LRU list. To be "pinned" means ** that the page is in active use and must not be deallocated. @@ -576,7 +604,7 @@ static void pcache1ResizeHash(PCache1 *p){ ** ** The PGroup mutex must be held when this function is called. */ -static PgHdr1 *pcache1PinPage(PgHdr1 *pPage){ +static sqlite3_pcache_page *pcache1PinPage(PgHdr1 *pPage){ assert( pPage!=0 ); assert( PAGE_IS_UNPINNED(pPage) ); assert( pPage->pLruNext ); @@ -590,7 +618,7 @@ static PgHdr1 *pcache1PinPage(PgHdr1 *pPage){ assert( pPage->isAnchor==0 ); assert( pPage->pCache->pGroup->lru.isAnchor==1 ); pPage->pCache->nRecyclable--; - return pPage; + return &pPage->page; } @@ -873,7 +901,7 @@ static int pcache1Pagecount(sqlite3_pcache *p){ ** usually not needed, and by avoiding the stack initialization required ** for these steps, the main pcache1Fetch() procedure can run faster. */ -static SQLITE_NOINLINE PgHdr1 *pcache1FetchStage2( +static SQLITE_NOINLINE sqlite3_pcache_page *pcache1FetchStage2( PCache1 *pCache, unsigned int iKey, int createFlag @@ -939,8 +967,9 @@ static SQLITE_NOINLINE PgHdr1 *pcache1FetchStage2( if( iKey>pCache->iMaxKey ){ pCache->iMaxKey = iKey; } + return &pPage->page; } - return pPage; + return 0; } /* @@ -1002,7 +1031,7 @@ static SQLITE_NOINLINE PgHdr1 *pcache1FetchStage2( ** the common case where pGroup->mutex is NULL. The pcache1Fetch() wrapper ** invokes the appropriate routine. */ -static PgHdr1 *pcache1FetchNoMutex( +static sqlite3_pcache_page *pcache1FetchNoMutex( sqlite3_pcache *p, unsigned int iKey, int createFlag @@ -1025,7 +1054,7 @@ static PgHdr1 *pcache1FetchNoMutex( if( PAGE_IS_UNPINNED(pPage) ){ return pcache1PinPage(pPage); }else{ - return pPage; + return &pPage->page; } }else if( createFlag ){ /* Steps 3, 4, and 5 implemented by this subroutine */ @@ -1035,13 +1064,13 @@ static PgHdr1 *pcache1FetchNoMutex( } } #if PCACHE1_MIGHT_USE_GROUP_MUTEX -static PgHdr1 *pcache1FetchWithMutex( +static sqlite3_pcache_page *pcache1FetchWithMutex( sqlite3_pcache *p, unsigned int iKey, int createFlag ){ PCache1 *pCache = (PCache1 *)p; - PgHdr1 *pPage; + sqlite3_pcache_page *pPage; pcache1EnterMutex(pCache->pGroup); pPage = pcache1FetchNoMutex(p, iKey, createFlag); @@ -1059,7 +1088,6 @@ static sqlite3_pcache_page *pcache1Fetch( PCache1 *pCache = (PCache1 *)p; #endif - assert( offsetof(PgHdr1,page)==0 ); assert( pCache->bPurgeable || createFlag!=1 ); assert( pCache->bPurgeable || pCache->nMin==0 ); assert( pCache->bPurgeable==0 || pCache->nMin==10 ); @@ -1067,11 +1095,11 @@ static sqlite3_pcache_page *pcache1Fetch( assert( pCache->nHash>0 ); #if PCACHE1_MIGHT_USE_GROUP_MUTEX if( pCache->pGroup->mutex ){ - return (sqlite3_pcache_page*)pcache1FetchWithMutex(p, iKey, createFlag); + return pcache1FetchWithMutex(p, iKey, createFlag); }else #endif { - return (sqlite3_pcache_page*)pcache1FetchNoMutex(p, iKey, createFlag); + return pcache1FetchNoMutex(p, iKey, createFlag); } } @@ -1087,7 +1115,7 @@ static void pcache1Unpin( int reuseUnlikely ){ PCache1 *pCache = (PCache1 *)p; - PgHdr1 *pPage = (PgHdr1 *)pPg; + PgHdr1 *pPage = PAGE_TO_PGHDR1(pPg); PGroup *pGroup = pCache->pGroup; assert( pPage->pCache==pCache ); @@ -1123,7 +1151,7 @@ static void pcache1Rekey( unsigned int iNew ){ PCache1 *pCache = (PCache1 *)p; - PgHdr1 *pPage = (PgHdr1 *)pPg; + PgHdr1 *pPage = PAGE_TO_PGHDR1(pPg); PgHdr1 **pp; unsigned int hOld, hNew; assert( pPage->iKey==iOld ); @@ -1132,7 +1160,7 @@ static void pcache1Rekey( pcache1EnterMutex(pCache->pGroup); - assert( pcache1FetchNoMutex(p, iOld, 0)==pPage ); /* pPg really is iOld */ + assert( pcache1FetchNoMutex(p, iOld, 0)==&pPage->page ); /* pPg really iOld */ assert( pCache->nHash>0 && (pCache->nHash & (pCache->nHash-1))==0 ); hOld = iOld & (pCache->nHash-1); pp = &pCache->apHash[hOld]; diff --git a/test/pcacheleak.test b/test/pcacheleak.test new file mode 100644 index 0000000000..1d2c633b40 --- /dev/null +++ b/test/pcacheleak.test @@ -0,0 +1,96 @@ +# 2026 October 24 +# +# The author disclaims copyright to this source code. In place of +# a legal notice, here is a blessing: +# +# May you do good and not evil. +# May you find forgiveness for yourself and forgive others. +# May you share freely, never taking more than you give. +# +#*********************************************************************** +# +# This file implements a regression test for an information-leak primitive +# that exploits the b-tree page overread. +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +set testprefix pcacheleak + +database_may_be_corrupt + +# Build the corrupted db. +# +proc build_leak_db {filename} { + forcedelete $filename + sqlite3 dbx $filename + dbx eval { + PRAGMA page_size=4096; + PRAGMA auto_vacuum=0; + CREATE TABLE users(id INTEGER PRIMARY KEY, name TEXT); + INSERT INTO users(id,name) VALUES(1,'Alice'); + INSERT INTO users(id,name) VALUES(2,'Bob'); + } + dbx close + + # Modification 1: bump the page-2 cell count from 2 to 3. The 2-byte + # cell count is at page offset 3; page 2 starts at file offset 0x1000, + # so the low byte is at 0x1004 == 4100. + hexio_write $filename 4100 03 + + # Modification 2: write a third cell pointer into the cell pointer array + # (which begins at page offset 8, file offset 0x1008). The two real + # pointers occupy 0x1008 and 0x100A; the fake third pointer goes at + # 0x100C == 4108 and is set to the page-relative offset 0x0FF8, i.e. the + # last 8 bytes of the 4096-byte page. + hexio_write $filename 4108 0ff8 + + # Modification 3: overwrite those last 8 bytes (0x1FF8 == 8184) with + # 0xFF. Every byte has its high bit set, so the varint parser treats + # them all as continuation bytes and is forced to read past the end of + # the page buffer to find the end of the number. + hexio_write $filename 8184 ffffffffffffffff +} + +# The fake cell is parsed, so the table now appears to hold three rows +# instead of two. +# +do_test 1.0 { + build_leak_db test.db + sqlite3 db test.db + db eval {SELECT count(*) FROM users} +} {3} + + +# Turning the cell size check on causes the same file to be rejected as +# corrupt, because the fake cell is detected before it can be parsed. +# +do_test 1.1 { + sqlite3 db test.db + db eval {PRAGMA cell_size_check=ON} + list [catch {db eval {SELECT rowid FROM users}} msg] $msg +} {1 {database disk image is malformed}} +db close + +# The leak itself. Make four byte-identical copies of the manipulated +# database and open them all at once. Each open allocates its own page-2 +# buffer, and because all four buffers are live simultaneously they must +# occupy four different heap addresses. If the leaked rowid contains +# heap address bytes, all four values should be distinct. Otherwise, +# they should all be identical (and either 0 or 1) +# +do_test 2.0 { + set vals [list] + foreach h {d0 d1 d2 d3} { + forcecopy test.db test.db.$h + sqlite3 $h test.db.$h + lappend vals [lindex [$h eval {SELECT rowid FROM users}] end] + } + foreach h {d0 d1 d2 d3} { $h close } + + # More than two distinct values (0 and 1) proves the leaked rowid + # the buffer's memory address rather than on the (identical) file contents. + list [llength [lsort -unique $vals]] [lindex $vals 0] +} {1 0} + +finish_test