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
This commit is contained in:
dan committed 2026-10-24 14:37:18 +00:00
1 parent 9a9446f3b0
commit 98fced6200
4 files changed
+150 -24

No files matched your search

+8 -6
View File
@@ -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.
+1 -1
View File
@@ -1 +1 @@
e2e7c8eb02daea0876fb25abccd96c769f55fba3e995f0e7cf29aa992d7c61a6
3f9a0ba4f4dffac7a4b9de30e66d94498a1433d96e327cfff43134b8856f4ba1
+45 -17
View File
@@ -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];
+96
View File
@@ -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