From a1f7c0a21c65431a3fbfeba25ab7f1e9ad827273 Mon Sep 17 00:00:00 2001
From: drh <drh@noemail.net>
Date: Fri, 28 Mar 2014 03:12:48 +0000
Subject: [PATCH] Enhance the sqlite3VdbeRecordCompare() routines so that if
they encounter database corruption, they will set the
UnpackedRecord.isCorrupt field and return 0. The
sqlite3BtreeMovetoUnpacked() routine detects this and returns SQLITE_CORRUPT,
causing the corruption to be reported back to the top-level.
FossilOrigin-Name: 7fa85eaaaf6d211378620d728a759fdfe30a15b0
---
manifest | 24 ++++++++++++------------
manifest.uuid | 2 +-
src/btree.c | 3 +++
src/sqliteInt.h | 1 +
src/vdbe.h | 4 ++--
src/vdbeInt.h | 2 +-
src/vdbeaux.c | 27 +++++++++++++++++++--------
test/corruptG.test | 11 +++--------
test/corruptI.test | 2 +-
9 files changed, 43 insertions(+), 33 deletions(-)
diff --git a/manifest b/manifest
index c8233bfa12..48ca1421b2 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Minor\scleanup\sof\sthe\scode\sin\sthe\squery\splanner\sthat\scomputes\sthe\scosts\s\nestimates\sfor\sthe\svarious\splans.\s\sThere\sare\sno\schanges\sto\sthe\scosts\sat\sthis\ntime.\s\sBut\sthe\scode\sis\sslightly\smore\sreadable\snow\sand\sthat\smight\sfacilitate\nfuture\senhancements.
-D 2014-03-27T18:36:34.321
+C Enhance\sthe\ssqlite3VdbeRecordCompare()\sroutines\sso\sthat\sif\sthey\sencounter\ndatabase\scorruption,\sthey\swill\sset\sthe\sUnpackedRecord.isCorrupt\sfield\sand\nreturn\s0.\s\sThe\ssqlite3BtreeMovetoUnpacked()\sroutine\sdetects\sthis\sand\sreturns\nSQLITE_CORRUPT,\scausing\sthe\scorruption\sto\sbe\sreported\sback\sto\sthe\stop-level.
+D 2014-03-28T03:12:48.763
F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f
F Makefile.in 2ef13430cd359f7b361bb863504e227b25cc7f81
F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23
@@ -164,7 +164,7 @@ F src/auth.c 523da7fb4979469955d822ff9298352d6b31de34
F src/backup.c a729e63cf5cd1829507cb7b8e89f99b95141bb53
F src/bitvec.c 19a4ba637bd85f8f63fc8c9bae5ade9fb05ec1cb
F src/btmutex.c 976f45a12e37293e32cae0281b15a21d48a8aaa7
-F src/btree.c 8d7e432bdd27d63182865c708ea0e7606489b6d1
+F src/btree.c a59a199f21338ae1847d69f5db87c3e8ef1b1578
F src/btree.h 232836cb51753f2e96aa8ce0f052c6df850f76ba
F src/btreeInt.h 0be66063468a520e4d66b80c7a1dc26d04ee6ea4
F src/build.c 0d50ef95aad63f4c4fc47f3fa2670d4557c45db0
@@ -222,7 +222,7 @@ F src/shell.c cee9f46f2688a261601b1fd3d7f4b3cddf9b5cdf
F src/sqlite.h.in a2ef671f92747a5a1c8a47bad5c585a8dd9eca80
F src/sqlite3.rc 11094cc6a157a028b301a9f06b3d03089ea37c3e
F src/sqlite3ext.h 886f5a34de171002ad46fae8c36a7d8051c190fc
-F src/sqliteInt.h afbf39e96736ceb85e1d896b281ba2406dd70aa0
+F src/sqliteInt.h cc9582a91b2910404ccda7b7e198815ea0f75948
F src/sqliteLimit.h 164b0e6749d31e0daa1a4589a169d31c0dec7b3d
F src/status.c 7ac05a5c7017d0b9f0b4bcd701228b784f987158
F src/table.c 2cd62736f845d82200acfa1287e33feb3c15d62e
@@ -279,10 +279,10 @@ F src/utf.c 6dc9ec9f1b3db43ae8ba0365377f11df1ee4c01c
F src/util.c c46c90459ef9bdc0c6c73803cf4c55425b4771cf
F src/vacuum.c 3728d74919d4fb1356f9e9a13e27773db60b7179
F src/vdbe.c 74c7386e83eee56f921a17bb4a0396c9551f5bc7
-F src/vdbe.h fb2c48c198300a7c632f09fc940011d2ad2fc2ae
-F src/vdbeInt.h 2b9a6849166d0014c843ae3fd83a062be4efa325
+F src/vdbe.h 394464909ed682334aa3d5831aae0c2fe2abef94
+F src/vdbeInt.h e6d83e5bfd62fc6685ba1ed6153f7099f82de9f7
F src/vdbeapi.c 0ed6053f947edd0b30f64ce5aeb811872a3450a4
-F src/vdbeaux.c f81ef920dcf76aceaa1ce77081e9fc5d7a0993dd
+F src/vdbeaux.c 1153175fb57a8454e1c8cf79b59b7bf92b26779d
F src/vdbeblob.c 15377abfb59251bccedd5a9c7d014a895f0c04aa
F src/vdbemem.c 6fc77594c60f6155404f3f8d71bf36d1fdeb4447
F src/vdbesort.c 4abb7c0f8f19b7d7d82f4558d5da1a30fdf9ea38
@@ -403,9 +403,9 @@ F test/corruptC.test 02405cf7ed0c1e989060e1aab6d02ffbc3906fbb
F test/corruptD.test b3c205fac7952b1de645ce44bb02335cd9e3e040
F test/corruptE.test 193b4ca4e927e77c1d5f4f56203ddc998432a7ee
F test/corruptF.test be9fde98e4c93648f1ba52b74e5318edc8f59fe4
-F test/corruptG.test 58ec333a01997fe655e34e5bea52b7a2a6b9704d
+F test/corruptG.test 1ab3bf97ee7bdba70e0ff3ba2320657df55d1804
F test/corruptH.test 88ed71a086e13591c917aac6de32750e7c7281cb
-F test/corruptI.test 1b796461e5b635e0a74e3c4ecb1121c82d319dff
+F test/corruptI.test b3e4203d420490fc3d3062711597bc1dea06a789
F test/count.test 42a251178e32f617eda33f76236a7f79825a50b5
F test/coveridxscan.test cdb47d01acc4a634a34fd25abe85189e0d0f1e62
F test/crash.test fb9dc4a02dcba30d4aa5c2c226f98b220b2b959f
@@ -1159,7 +1159,7 @@ F tool/vdbe_profile.tcl 67746953071a9f8f2f668b73fe899074e2c6d8c1
F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4
F tool/warnings.sh d1a6de74685f360ab718efda6265994b99bbea01
F tool/win/sqlite.vsix 030f3eeaf2cb811a3692ab9c14d021a75ce41fff
-P b4bd2a062c4baf5f622d61b7411f00de5904ef56
-R 96966a646967b8a2388d32a924ac0ae2
+P 9b4d7226bcee38be5ac68a54bee03b4179cb69fc
+R e6ff5922142fb694f528da1c1b2a1baf
U drh
-Z 308d20d60c0618b1b3ae79dc397c2638
+Z 2ba581858b0e7a94365bc5b4fc44e179
diff --git a/manifest.uuid b/manifest.uuid
index 65bd2a013d..ef58fd2f6f 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-9b4d7226bcee38be5ac68a54bee03b4179cb69fc
\ No newline at end of file
+7fa85eaaaf6d211378620d728a759fdfe30a15b0
\ No newline at end of file
diff --git a/src/btree.c b/src/btree.c
index 43d41d67e9..c3055836c9 100644
--- a/src/btree.c
+++ b/src/btree.c
@@ -4588,6 +4588,7 @@ int sqlite3BtreeMovetoUnpacked(
if( pIdxKey ){
xRecordCompare = sqlite3VdbeFindCompare(pIdxKey);
+ pIdxKey->isCorrupt = 0;
assert( pIdxKey->default_rc==1
|| pIdxKey->default_rc==0
|| pIdxKey->default_rc==-1
@@ -4711,6 +4712,7 @@ int sqlite3BtreeMovetoUnpacked(
c = xRecordCompare(nCell, pCellKey, pIdxKey, 0);
sqlite3_free(pCellKey);
}
+ assert( pIdxKey->isCorrupt==0 || c==0 );
if( c<0 ){
lwr = idx+1;
}else if( c>0 ){
@@ -4720,6 +4722,7 @@ int sqlite3BtreeMovetoUnpacked(
*pRes = 0;
rc = SQLITE_OK;
pCur->aiIdx[pCur->iPage] = (u16)idx;
+ if( pIdxKey->isCorrupt ) rc = SQLITE_CORRUPT;
goto moveto_finish;
}
if( lwr>upr ) break;
diff --git a/src/sqliteInt.h b/src/sqliteInt.h
index 5e0dd91ec9..0f67748fc8 100644
--- a/src/sqliteInt.h
+++ b/src/sqliteInt.h
@@ -1627,6 +1627,7 @@ struct UnpackedRecord {
KeyInfo *pKeyInfo; /* Collation and sort-order information */
u16 nField; /* Number of entries in apMem[] */
i8 default_rc; /* Comparison result if keys are equal */
+ u8 isCorrupt; /* Corruption detected by xRecordCompare() */
Mem *aMem; /* Values */
int r1; /* Value to return if (lhs > rhs) */
int r2; /* Value to return if (rhs < lhs) */
diff --git a/src/vdbe.h b/src/vdbe.h
index 8e300b88a6..10a4140ee1 100644
--- a/src/vdbe.h
+++ b/src/vdbe.h
@@ -211,10 +211,10 @@ void sqlite3VdbeSetVarmask(Vdbe*, int);
#endif
void sqlite3VdbeRecordUnpack(KeyInfo*,int,const void*,UnpackedRecord*);
-int sqlite3VdbeRecordCompare(int,const void*,const UnpackedRecord*,int);
+int sqlite3VdbeRecordCompare(int,const void*,UnpackedRecord*,int);
UnpackedRecord *sqlite3VdbeAllocUnpackedRecord(KeyInfo *, char *, int, char **);
-typedef int (*RecordCompare)(int,const void*,const UnpackedRecord*,int);
+typedef int (*RecordCompare)(int,const void*,UnpackedRecord*,int);
RecordCompare sqlite3VdbeFindCompare(UnpackedRecord*);
#ifndef SQLITE_OMIT_TRIGGER
diff --git a/src/vdbeInt.h b/src/vdbeInt.h
index b752478035..043b56da59 100644
--- a/src/vdbeInt.h
+++ b/src/vdbeInt.h
@@ -392,7 +392,7 @@ u32 sqlite3VdbeSerialGet(const unsigned char*, u32, Mem*);
void sqlite3VdbeDeleteAuxData(Vdbe*, int, int);
int sqlite2BtreeKeyCompare(BtCursor *, const void *, int, int, int *);
-int sqlite3VdbeIdxKeyCompare(VdbeCursor*,const UnpackedRecord*,int*);
+int sqlite3VdbeIdxKeyCompare(VdbeCursor*,UnpackedRecord*,int*);
int sqlite3VdbeIdxRowid(sqlite3*, BtCursor *, i64 *);
int sqlite3MemCompare(const Mem*, const Mem*, const CollSeq*);
int sqlite3VdbeExec(Vdbe*);
diff --git a/src/vdbeaux.c b/src/vdbeaux.c
index f5e4b0a9fc..0ce21378d5 100644
--- a/src/vdbeaux.c
+++ b/src/vdbeaux.c
@@ -3405,10 +3405,13 @@ static i64 vdbeRecordDecodeInt(u32 serial_type, const u8 *aKey){
** Key1 and Key2 do not have to contain the same number of fields. If all
** fields that appear in both keys are equal, then pPKey2->default_rc is
** returned.
+**
+** If database corruption is discovered, set pPKey2->isCorrupt to non-zero
+** and return 0.
*/
int sqlite3VdbeRecordCompare(
int nKey1, const void *pKey1, /* Left key */
- const UnpackedRecord *pPKey2, /* Right key */
+ UnpackedRecord *pPKey2, /* Right key */
int bSkip /* If true, skip the first field */
){
u32 d1; /* Offset into aKey[] of next data element */
@@ -3434,7 +3437,10 @@ int sqlite3VdbeRecordCompare(
}else{
idx1 = getVarint32(aKey1, szHdr1);
d1 = szHdr1;
- if( d1>(unsigned)nKey1 ) return 1; /* Corruption */
+ if( d1>(unsigned)nKey1 ){
+ pPKey2->isCorrupt = (u8)SQLITE_CORRUPT_BKPT;
+ return 0; /* Corruption */
+ }
i = 0;
}
@@ -3511,7 +3517,8 @@ int sqlite3VdbeRecordCompare(
testcase( (d1+mem1.n)==(unsigned)nKey1 );
testcase( (d1+mem1.n+1)==(unsigned)nKey1 );
if( (d1+mem1.n) > (unsigned)nKey1 ){
- rc = 1; /* Corruption */
+ pPKey2->isCorrupt = (u8)SQLITE_CORRUPT_BKPT;
+ return 0; /* Corruption */
}else if( pKeyInfo->aColl[i] ){
mem1.enc = pKeyInfo->enc;
mem1.db = pKeyInfo->db;
@@ -3537,7 +3544,8 @@ int sqlite3VdbeRecordCompare(
testcase( (d1+nStr)==(unsigned)nKey1 );
testcase( (d1+nStr+1)==(unsigned)nKey1 );
if( (d1+nStr) > (unsigned)nKey1 ){
- rc = 1; /* Corruption */
+ pPKey2->isCorrupt = (u8)SQLITE_CORRUPT_BKPT;
+ return 0; /* Corruption */
}else{
int nCmp = MIN(nStr, pRhs->n);
rc = memcmp(&aKey1[d1], pRhs->z, nCmp);
@@ -3596,7 +3604,7 @@ int sqlite3VdbeRecordCompare(
*/
static int vdbeRecordCompareInt(
int nKey1, const void *pKey1, /* Left key */
- const UnpackedRecord *pPKey2, /* Right key */
+ UnpackedRecord *pPKey2, /* Right key */
int bSkip /* Ignored */
){
const u8 *aKey = &((const u8*)pKey1)[*(const u8*)pKey1 & 0x3F];
@@ -3694,7 +3702,7 @@ static int vdbeRecordCompareInt(
*/
static int vdbeRecordCompareString(
int nKey1, const void *pKey1, /* Left key */
- const UnpackedRecord *pPKey2, /* Right key */
+ UnpackedRecord *pPKey2, /* Right key */
int bSkip
){
const u8 *aKey1 = (const u8*)pKey1;
@@ -3715,7 +3723,10 @@ static int vdbeRecordCompareString(
int szHdr = aKey1[0];
nStr = (serial_type-12) / 2;
- if( (szHdr + nStr) > nKey1 ) return 0; /* Corruption */
+ if( (szHdr + nStr) > nKey1 ){
+ pPKey2->isCorrupt = (u8)SQLITE_CORRUPT_BKPT;
+ return 0; /* Corruption */
+ }
nCmp = MIN( pPKey2->aMem[0].n, nStr );
res = memcmp(&aKey1[szHdr], pPKey2->aMem[0].z, nCmp);
@@ -3880,7 +3891,7 @@ idx_rowid_corruption:
*/
int sqlite3VdbeIdxKeyCompare(
VdbeCursor *pC, /* The cursor to compare against */
- const UnpackedRecord *pUnpacked, /* Unpacked version of key */
+ UnpackedRecord *pUnpacked, /* Unpacked version of key */
int *res /* Write the comparison result here */
){
i64 nCellKey = 0;
diff --git a/test/corruptG.test b/test/corruptG.test
index 1ec5d6f6a9..af920edf41 100644
--- a/test/corruptG.test
+++ b/test/corruptG.test
@@ -47,12 +47,12 @@ do_test 1.2 {
catchsql {
SELECT c FROM t1 WHERE a>'abc';
}
-} {0 {}}
+} {1 {database disk image is malformed}}
do_test 1.3 {
catchsql {
PRAGMA integrity_check
}
-} {0 ok}
+} {1 {database disk image is malformed}}
do_test 1.4 {
catchsql {
SELECT c FROM t1 ORDER BY a;
@@ -71,11 +71,6 @@ do_test 2.1 {
catchsql {
SELECT rowid FROM t1 WHERE a='abc' and b='xyz123456789XYZ';
}
- # The following test result is brittle. The point above is to try to
- # force a buffer overread by a corrupt database file. If we get an
- # incorrect answer from a corrupt database file, that is OK. If the
- # result below changes, that just means that "undefined behavior" has
- # changed.
-} {/0 .*/}
+} {1 {database disk image is malformed}}
finish_test
diff --git a/test/corruptI.test b/test/corruptI.test
index 087a0f3b01..ed34c0f8c3 100644
--- a/test/corruptI.test
+++ b/test/corruptI.test
@@ -51,7 +51,7 @@ do_test 1.3 {
hexio_write test.db $off FFFF7f02
sqlite3 db test.db
catchsql { SELECT * FROM t1 WHERE a = 10 }
-} {0 {}}
+} {1 {database disk image is malformed}}
do_test 2.0 {
execsql {