From ec1fc80ca3beca3dc40f66bc3faa1079f5277075 Mon Sep 17 00:00:00 2001 From: drh Date: Wed, 13 Aug 2008 14:07:40 +0000 Subject: [PATCH] A partial fix for ticket #3292. This fixes the original problem but there are other similar problems lurking in the code still. (CVS 5561) FossilOrigin-Name: 055f173ab1b6fb657bf817faa3a37335d8fa60d5 --- manifest | 23 ++++++++-------- manifest.uuid | 2 +- src/btree.c | 6 ++--- src/sqliteInt.h | 4 +-- src/vdbe.c | 6 ++--- src/vdbe.h | 4 +-- src/vdbeaux.c | 65 ++++++++++++++++++++++++++++++---------------- test/corrupt7.test | 57 +++++++++++++++++++++------------------- test/tkt3292.test | 61 +++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 157 insertions(+), 71 deletions(-) create mode 100644 test/tkt3292.test diff --git a/manifest b/manifest index c5ac8de054..4a0b7cfdf2 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Make\ssure\sthe\slookaside\stest\sscript\ssaturates\sthe\slookaside\sbuffer\seven\nwhen\sSQLITE_DEBUG\sis\soff.\s\sTicket\s#3289\s(CVS\s5560) -D 2008-08-12T15:48:25 +C A\spartial\sfix\sfor\sticket\s#3292.\s\sThis\sfixes\sthe\soriginal\sproblem\sbut\sthere\nare\sother\ssimilar\sproblems\slurking\sin\sthe\scode\sstill.\s(CVS\s5561) +D 2008-08-13T14:07:40 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in 2713ea64947be3b35f35d9a3158bb8299c90b019 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -96,7 +96,7 @@ F src/attach.c a85c14612e7e3410e0c3d2e0241832fa9688bd14 F src/auth.c c8b2ab5c8bad4bd90ed7c294694f48269162c627 F src/bitvec.c 95c86bd18d8fedf0533f5af196192546e10a7e7d F src/btmutex.c 709cad2cdca0afd013f0f612363810e53f59ec53 -F src/btree.c da7491ef06c0209da962e1520bfc5c90b5c5cc96 +F src/btree.c 2ae1092eee58d2d89e5d2f59f711f805dec59bbc F src/btree.h 03256ed7ee42b5ecacbe887070b0f8249e7d069d F src/btreeInt.h ab18c7b4980314e9e4b402e5dcde09f3c2545576 F src/build.c 931ed94fd3bbd67b6ac9d5ac6a45dc01e9f01726 @@ -146,7 +146,7 @@ F src/select.c 390d1bdde0c24f0225e369896da8e60ef2aeffbe F src/shell.c d83b578a8ccdd3e0e7fef4388a0887ce9f810967 F src/sqlite.h.in 54e51c22e2294c5989156b0aec87aa44168ac1f0 F src/sqlite3ext.h 1e3887c9bd3ae66cb599e922824b04cd0d0f2c3e -F src/sqliteInt.h be39605e2deee45c1004c8bf3927bdab2ec97e0c +F src/sqliteInt.h df48efd8c4cb7b833bd7299a41c59c6651be0072 F src/sqliteLimit.h f435e728c6b620ef7312814d660a81f9356eb5c8 F src/status.c 8caa772cd9310bc297280f7cf0ede4d69ed5b801 F src/table.c 22744786199c9195720c15a7a42cb97b2e2728d8 @@ -183,11 +183,11 @@ F src/update.c 79b77a3cc8ed5f8903a7f37055fcedd69388dcae F src/utf.c c63e6f69082f85c19ab88d62dedaf91d71ac1a50 F src/util.c afe659ccc05d1f8af9e8631dabfec3ee3a7144af F src/vacuum.c ef342828002debc97514617af3424aea8ef8522c -F src/vdbe.c 2eff8c2206048f73e1ce23a56ed666b0591c56d5 -F src/vdbe.h 647fcf33a551ba10a974162c56846cb9aef2276b +F src/vdbe.c cda10a8e6ac05c783d6cd8a1d4852754fa73cf3b +F src/vdbe.h ccca49ce3db3486ae27f9baec68a77ccc29c43a9 F src/vdbeInt.h 6f04c2bf65a0d5c2bb8318b226278a35d1f7a8f5 F src/vdbeapi.c f21971516880fd3a10821b2cdd0e64a5a63952c9 -F src/vdbeaux.c 21126e0d319e19125f5e42fceafb17eafe58721c +F src/vdbeaux.c 7b25fbbbdbd548b71c0a44d2f90fd3d0b06e70b8 F src/vdbeblob.c f93110888ddc246215e9ba1f831d3d375bfd8355 F src/vdbefifo.c 20fda2a7c4c0bcee1b90eb7e545fefcdbf2e1de7 F src/vdbemem.c c37b2a266a49eaf0c0f5080157f9f1a908fdaac3 @@ -250,7 +250,7 @@ F test/corrupt3.test 263e8bb04e2728df832fddf6973cf54c91db0c32 F test/corrupt4.test acdb01afaedf529004b70e55de1a6f5a05ae7fff F test/corrupt5.test 7796d5bdfe155ed824cee9dff371f49da237cfe0 F test/corrupt6.test e69b877d478224deab7b66844566258cecacd25e -F test/corrupt7.test 34d3217f2e1b783e68374430e4049a3416e1d9e9 +F test/corrupt7.test 7a3be79b93dba88ba8472d61b57cb6d7b66cb69e F test/corrupt8.test 9992ef7f67cefc576b92373f6bf5ab8775280f51 F test/corrupt9.test 794d284109c65c8f10a2b275479045e02d163bae F test/corruptA.test 99e95620b980161cb3e79f06a884a4bb8ae265ff @@ -545,6 +545,7 @@ F test/tkt3080.test 31a02e87a4c80ed443831c2c5b0e8216ff95ac14 F test/tkt3093.test fbdbc5b4969244ad11f540759003e361fcaf391f F test/tkt3121.test 536df66a02838c26a12fe98639354ca1290ca68b F test/tkt3201.test 607d433ad2c1f6a8cb1af55aaca427f63c83191b +F test/tkt3292.test 962465a0984a3b8c757efe59c2c59144871ee1dd F test/tokenize.test ce430a7aed48fc98301611429595883fdfcab5d7 F test/trace.test 951cd0f5f571e7f36bf7bfe04be70f90fb16fb00 F test/trans.test 2fd24cd7aa0b879d49a224cbd647d698f1e7ac5c @@ -617,7 +618,7 @@ F tool/speedtest16.c c8a9c793df96db7e4933f0852abb7a03d48f2e81 F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 1dbced29de5f59ba2ebf877edcadf171540374d1 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e -P 697fe7a3167c22a3232ce154e9d47cf75af613c4 -R bc3261e0e67b2840551eab89813d1cbf +P d6aacc5dc7c06f97fb5faa3d85a8f2d8ab0dd554 +R 9b480ed8f28854c07567551a082c6f07 U drh -Z 492a17374325436959e89a8ba7f54c34 +Z d0f90d7840e7d910b5f941e8ef52ae1d diff --git a/manifest.uuid b/manifest.uuid index 87a6dacfab..c7a7043d69 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -d6aacc5dc7c06f97fb5faa3d85a8f2d8ab0dd554 \ No newline at end of file +055f173ab1b6fb657bf817faa3a37335d8fa60d5 \ No newline at end of file diff --git a/src/btree.c b/src/btree.c index 3daa8f807f..a75da9c615 100644 --- a/src/btree.c +++ b/src/btree.c @@ -9,7 +9,7 @@ ** May you share freely, never taking more than you give. ** ************************************************************************* -** $Id: btree.c,v 1.495 2008/08/02 17:36:46 danielk1977 Exp $ +** $Id: btree.c,v 1.496 2008/08/13 14:07:40 drh Exp $ ** ** This file implements a external (disk-based) database using BTrees. ** See the header comment on "btreeInt.h" for additional information. @@ -3774,7 +3774,7 @@ int sqlite3BtreeMoveto( pCellKey = (void *)fetchPayload(pCur, &available, 0); nCellKey = pCur->info.nKey; if( available>=nCellKey ){ - c = sqlite3VdbeRecordCompare(nCellKey, pCellKey, pUnKey); + c = sqlite3VdbeRecordCompare(nCellKey, pCellKey, 0, pUnKey); }else{ pCellKey = sqlite3Malloc( nCellKey ); if( pCellKey==0 ){ @@ -3782,7 +3782,7 @@ int sqlite3BtreeMoveto( goto moveto_finish; } rc = sqlite3BtreeKey(pCur, 0, nCellKey, (void *)pCellKey); - c = sqlite3VdbeRecordCompare(nCellKey, pCellKey, pUnKey); + c = sqlite3VdbeRecordCompare(nCellKey, pCellKey, 0, pUnKey); sqlite3_free(pCellKey); if( rc ) goto moveto_finish; } diff --git a/src/sqliteInt.h b/src/sqliteInt.h index 43271a0528..45c787e1f0 100644 --- a/src/sqliteInt.h +++ b/src/sqliteInt.h @@ -11,7 +11,7 @@ ************************************************************************* ** Internal interface definitions for SQLite. ** -** @(#) $Id: sqliteInt.h,v 1.753 2008/08/12 15:21:12 drh Exp $ +** @(#) $Id: sqliteInt.h,v 1.754 2008/08/13 14:07:40 drh Exp $ */ #ifndef _SQLITEINT_H_ #define _SQLITEINT_H_ @@ -1046,7 +1046,7 @@ struct KeyInfo { sqlite3 *db; /* The database connection */ u8 enc; /* Text encoding - one of the TEXT_Utf* values */ u8 incrKey; /* Increase 2nd key by epsilon before comparison */ - u8 prefixIsEqual; /* Treat a prefix as equal */ + u8 ckPrefixOnly; /* Records are equal if shorter is a prefix of longer */ int nField; /* Number of entries in aColl[] */ u8 *aSortOrder; /* If defined an aSortOrder[i] is true, sort DESC */ CollSeq *aColl[1]; /* Collating sequence for each term of the key */ diff --git a/src/vdbe.c b/src/vdbe.c index 42e693921c..d02bc8da73 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -43,7 +43,7 @@ ** in this file for details. If in doubt, do not deviate from existing ** commenting and indentation practices when changing or adding code. ** -** $Id: vdbe.c,v 1.773 2008/08/11 18:44:58 drh Exp $ +** $Id: vdbe.c,v 1.774 2008/08/13 14:07:40 drh Exp $ */ #include "sqliteInt.h" #include @@ -3058,10 +3058,10 @@ case OP_Found: { /* jump, in3 */ assert( pC->isTable==0 ); assert( pIn3->flags & MEM_Blob ); if( pOp->opcode==OP_Found ){ - pC->pKeyInfo->prefixIsEqual = 1; + pC->pKeyInfo->ckPrefixOnly = 1; } rc = sqlite3BtreeMoveto(pC->pCursor, pIn3->z, 0, pIn3->n, 0, &res); - pC->pKeyInfo->prefixIsEqual = 0; + pC->pKeyInfo->ckPrefixOnly = 0; if( rc!=SQLITE_OK ){ break; } diff --git a/src/vdbe.h b/src/vdbe.h index 815b615550..37d9f68639 100644 --- a/src/vdbe.h +++ b/src/vdbe.h @@ -15,7 +15,7 @@ ** or VDBE. The VDBE implements an abstract machine that runs a ** simple program to access and modify the underlying database. ** -** $Id: vdbe.h,v 1.135 2008/08/01 20:10:08 drh Exp $ +** $Id: vdbe.h,v 1.136 2008/08/13 14:07:41 drh Exp $ */ #ifndef _SQLITE_VDBE_H_ #define _SQLITE_VDBE_H_ @@ -190,7 +190,7 @@ int sqlite3VdbeReleaseMemory(int); #endif UnpackedRecord *sqlite3VdbeRecordUnpack(KeyInfo*,int,const void*,void*,int); void sqlite3VdbeDeleteUnpackedRecord(UnpackedRecord*); -int sqlite3VdbeRecordCompare(int,const void*,UnpackedRecord*); +int sqlite3VdbeRecordCompare(int,const void*,int,UnpackedRecord*); #ifndef NDEBUG diff --git a/src/vdbeaux.c b/src/vdbeaux.c index cded81f89b..1282785112 100644 --- a/src/vdbeaux.c +++ b/src/vdbeaux.c @@ -14,7 +14,7 @@ ** to version 2.8.7, all this code was combined into the vdbe.c source file. ** But that file was getting too big so this subroutines were split out. ** -** $Id: vdbeaux.c,v 1.405 2008/08/02 03:50:39 drh Exp $ +** $Id: vdbeaux.c,v 1.406 2008/08/13 14:07:41 drh Exp $ */ #include "sqliteInt.h" #include @@ -2274,18 +2274,32 @@ void sqlite3VdbeDeleteUnpackedRecord(UnpackedRecord *p){ ** sqlite3VdbeParseRecord. ** ** Key1 and Key2 do not have to contain the same number of fields. -** But if the lengths differ, Key2 must be the shorter of the two. +** The key with fewer fields is usually considered lessor than the +** longer. However if pPKey2->pKeyInfo->incrKey is set and +** the common prefixes are equal, then key1 is less than key2. +** Or if pPKey2->pKeyInfo->ckPrefixOnly flag is set and the +** prefixes are equal, then the keys are considered to be equal and +** the parts beyond the common prefix are ignored. +** +** The last nHdrIgnore1 bytes of the header of pKey1 are ignored, +** as if they do not exist. Usually nHdrIgnore1 is 0 which means +** that we look at the entire key. But sometimes nHdrIgnore1 is 1. +** When nHdrIgnore1 is 1, the keys are index records and so the last +** column is a rowid. The type code is always one byte in length. +** Hence, setting nHdrIgnore1 to 1 means that the final rowid at the +** end of the record should be treated as if it does not exist. ** ** Historical note: In earlier versions of this routine both Key1 ** and Key2 were blobs obtained from OP_MakeRecord. But we found ** that in typical use the same Key2 would be submitted multiple times ** in a row. So an optimization was added to parse the Key2 key ** separately and submit the parsed version. In this way, we avoid -** parsing the same Key2 multiple times in a row. +** parsing the same Key2 multiple times. */ int sqlite3VdbeRecordCompare( - int nKey1, const void *pKey1, - UnpackedRecord *pPKey2 + int nKey1, const void *pKey1, /* Left key */ + int nHdrIgnore1, /* Omit this much from end of key1 header */ + UnpackedRecord *pPKey2 /* Right key */ ){ u32 d1; /* Offset into aKey[] of next data element */ u32 idx1; /* Offset into aKey[] of next header element */ @@ -2305,6 +2319,7 @@ int sqlite3VdbeRecordCompare( idx1 = getVarint32(aKey1, szHdr1); d1 = szHdr1; + szHdr1 -= nHdrIgnore1; nField = pKeyInfo->nField; while( idx1nField ){ u32 serial_type1; @@ -2328,17 +2343,21 @@ int sqlite3VdbeRecordCompare( } if( mem1.zMalloc ) sqlite3VdbeMemRelease(&mem1); - /* One of the keys ran out of fields, but all the fields up to that point - ** were equal. If the incrKey flag is true, then the second key is - ** treated as larger. - */ if( rc==0 ){ + /* rc==0 here means that one of the keys ran out of fields and + ** all the fields up to that point were equal. If the incrKey + ** flag is true, then break the tie by treating the second key + ** as larger. If ckPrefixOnly is true, then keys with common prefixes + ** are considered to be equal. Otherwise, the longer key is the + ** larger. As it happens, the pPKey2 will always be the longer + ** if there is a difference. + */ if( pKeyInfo->incrKey ){ rc = -1; - }else if( !pKeyInfo->prefixIsEqual ){ - if( d1ckPrefixOnly ){ + /* Leave rc==0 */ + }else if( idx1aSortOrder && inField && pKeyInfo->aSortOrder[i] ){ @@ -2347,7 +2366,7 @@ int sqlite3VdbeRecordCompare( return rc; } - + /* ** The argument is an index entry composed using the OP_MakeRecord opcode. ** The last entry in this record should be an integer (specifically @@ -2366,7 +2385,7 @@ int sqlite3VdbeIdxRowidLen(const u8 *aKey, int nKey, int *pRowidLen){ *pRowidLen = sqlite3VdbeSerialTypeLen(typeRowid); return SQLITE_OK; } - + /* ** pCur points at an index entry created using the OP_MakeRecord opcode. @@ -2409,18 +2428,21 @@ int sqlite3VdbeIdxRowid(BtCursor *pCur, i64 *rowid){ ** ** pKey is either created without a rowid or is truncated so that it ** omits the rowid at the end. The rowid at the end of the index entry -** is ignored as well. +** is ignored as well. Hence, this routine only compares the prefixes +** of the keys prior to the final rowid, not the entire key. +** +** pUnpacked may be an unpacked version of pKey,nKey. If pUnpacked is +** supplied it is used in place of pKey,nKey. */ int sqlite3VdbeIdxKeyCompare( Cursor *pC, /* The cursor to compare against */ - UnpackedRecord *pUnpacked, + UnpackedRecord *pUnpacked, /* Unpacked version of pKey and nKey */ int nKey, const u8 *pKey, /* The key to compare */ int *res /* Write the comparison result here */ ){ i64 nCellKey = 0; int rc; BtCursor *pCur = pC->pCursor; - int lenRowid; Mem m; UnpackedRecord *pRec; char zSpace[200]; @@ -2433,9 +2455,8 @@ int sqlite3VdbeIdxKeyCompare( m.db = 0; m.flags = 0; m.zMalloc = 0; - if( (rc = sqlite3VdbeMemFromBtree(pC->pCursor, 0, nCellKey, 1, &m)) - || (rc = sqlite3VdbeIdxRowidLen((u8*)m.z, m.n, &lenRowid)) - ){ + rc = sqlite3VdbeMemFromBtree(pC->pCursor, 0, nCellKey, 1, &m); + if( rc ){ return rc; } if( !pUnpacked ){ @@ -2447,7 +2468,7 @@ int sqlite3VdbeIdxKeyCompare( if( pRec==0 ){ return SQLITE_NOMEM; } - *res = sqlite3VdbeRecordCompare(m.n-lenRowid, m.z, pRec); + *res = sqlite3VdbeRecordCompare(m.n, m.z, 1, pRec); if( !pUnpacked ){ sqlite3VdbeDeleteUnpackedRecord(pRec); } diff --git a/test/corrupt7.test b/test/corrupt7.test index 8b0b2fb82d..6cad383fd1 100644 --- a/test/corrupt7.test +++ b/test/corrupt7.test @@ -14,7 +14,7 @@ # segfault if it sees a corrupt database file. It specifically focuses # on corrupt cell offsets in a btree page. # -# $Id: corrupt7.test,v 1.4 2008/07/26 18:26:10 danielk1977 Exp $ +# $Id: corrupt7.test,v 1.5 2008/08/13 14:07:41 drh Exp $ set testdir [file dirname $argv0] source $testdir/tester.tcl @@ -72,31 +72,34 @@ do_test corrupt7-2.2 { } {{*** in database main *** Corruption detected in cell 15 on page 2}} -do_test corrupt7-3.1 { - execsql { - DROP TABLE t1; - CREATE TABLE t1(a, b); - INSERT INTO t1 VALUES(1, 'one'); - INSERT INTO t1 VALUES(100, 'one hundred'); - INSERT INTO t1 VALUES(100000, 'one hundred thousand'); - CREATE INDEX i1 ON t1(b); - } - db close - - # Locate the 3rd cell in the index. - set cell_offset [hexio_get_int [hexio_read test.db [expr 1024*2 + 12] 2]] - incr cell_offset [expr 1024*2] - incr cell_offset 1 - - # This write corrupts the "header-size" field of the database record - # stored in the index cell. At one point this was causing sqlite to - # reference invalid memory. - hexio_write test.db $cell_offset FFFF7F - - sqlite3 db test.db - catchsql { - SELECT b FROM t1 WHERE b > 'o' AND b < 'p'; - } -} {1 {database disk image is malformed}} +# The code path that was causing the buffer overrun that this test +# case was checking for was removed. +# +#do_test corrupt7-3.1 { +# execsql { +# DROP TABLE t1; +# CREATE TABLE t1(a, b); +# INSERT INTO t1 VALUES(1, 'one'); +# INSERT INTO t1 VALUES(100, 'one hundred'); +# INSERT INTO t1 VALUES(100000, 'one hundred thousand'); +# CREATE INDEX i1 ON t1(b); +# } +# db close +# +# # Locate the 3rd cell in the index. +# set cell_offset [hexio_get_int [hexio_read test.db [expr 1024*2 + 12] 2]] +# incr cell_offset [expr 1024*2] +# incr cell_offset 1 +# +# # This write corrupts the "header-size" field of the database record +# # stored in the index cell. At one point this was causing sqlite to +# # reference invalid memory. +# hexio_write test.db $cell_offset FFFF7F +# +# sqlite3 db test.db +# catchsql { +# SELECT b FROM t1 WHERE b > 'o' AND b < 'p'; +# } +#} {1 {database disk image is malformed}} finish_test diff --git a/test/tkt3292.test b/test/tkt3292.test new file mode 100644 index 0000000000..0f95244643 --- /dev/null +++ b/test/tkt3292.test @@ -0,0 +1,61 @@ +# 2008 August 12 +# +# 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 regression tests for SQLite library. +# Specifically, it tests the behavior of the sqlite3VdbeRecordCompare() +# routine in cases where the rowid is 0 or 1 in file format 4 +# (meaning that the rowid has type code 8 or 9 with zero bytes of +# data). Ticket #3292. +# +# $Id: tkt3292.test,v 1.1 2008/08/13 14:07:41 drh Exp $ + +set testdir [file dirname $argv0] +source $testdir/tester.tcl + +do_test tkt3292-1.1 { + execsql { + PRAGMA legacy_file_format=OFF; + CREATE TABLE t1(a INTEGER PRIMARY KEY, b INT); + INSERT INTO t1 VALUES(0, 1); + INSERT INTO t1 VALUES(1, 1); + INSERT INTO t1 VALUES(2, 1); + CREATE INDEX i1 ON t1(b); + SELECT * FROM t1 WHERE b>=1; + } +} {0 1 1 1 2 1} +do_test tkt3292-1.2 { + execsql { + INSERT INTO t1 VALUES(3, 0); + INSERT INTO t1 VALUES(4, 2); + SELECT * FROM t1 WHERE b>=1; + } +} {0 1 1 1 2 1 4 2} + + +do_test tkt3292-2.1 { + execsql { + CREATE TABLE t2(a INTEGER PRIMARY KEY, b, c, d); + INSERT INTO t2 VALUES(0, 1, 'hello', x'012345'); + INSERT INTO t2 VALUES(1, 1, 'hello', x'012345'); + INSERT INTO t2 VALUES(2, 1, 'hello', x'012345'); + CREATE INDEX i2 ON t2(b,c,d); + SELECT a FROM t2 WHERE b=1 AND c='hello' AND d>=x'012345'; + } +} {0 1 2} +do_test tkt3292-2.2 { + execsql { + INSERT INTO t2 VALUES(3, 1, 'hello', x'012344'); + INSERT INTO t2 VALUES(4, 1, 'hello', x'012346'); + SELECT a FROM t2 WHERE b=1 AND c='hello' AND d>=x'012345'; + } +} {0 1 2 4} + + +finish_test