Change the pcache1.c implementation so that the "header" occurs at the end

of page buffer, not at the beginning.  This insures that the 20 bytes
immediately following the page buffer are mapped if a read of the page
buffer overruns due to a malformed cell. (CVS 6711)

FossilOrigin-Name: c54de1f54080de7e134d7b562498abb5337a0a46

diff --git a/manifest b/manifest
index e399edde608bcc1d3308b8e51b406caca288999d..d1118341ec61db129729ccca2b7a470c722a2cbd 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Add\scorruptD.test,\sa\scontainer\sfor\stesting\sthe\s"cell\soverflow"\sproblem.\sAlso\sshuffle\sa\ssmall\samount\sof\scode\sin\sBtreeInitPage()\sto\scheck\sthat\sthe\spage\sheader\spointer\sto\sthe\sstart\sof\sthe\scell\soffset\sarray\sis\sset\sto\sa\ssane\svalue.\s(CVS\s6710)
-D 2009-06-03T17:26:18
+C Change\sthe\spcache1.c\simplementation\sso\sthat\sthe\s"header"\soccurs\sat\sthe\send\nof\spage\sbuffer,\snot\sat\sthe\sbeginning.\s\sThis\sinsures\sthat\sthe\s20\sbytes\nimmediately\sfollowing\sthe\spage\sbuffer\sare\smapped\sif\sa\sread\sof\sthe\spage\nbuffer\soverruns\sdue\sto\sa\smalformed\scell.\s(CVS\s6711)
+D 2009-06-03T21:04:36
 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0
 F Makefile.in 8b8fb7823264331210cddf103831816c286ba446
 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654
@@ -106,7 +106,7 @@ F src/auth.c 98db07c2088455797678eb1031f42d4d94d18a71
 F src/backup.c ff50af53184a5fd7bdee4d620b5dabef74717c79
 F src/bitvec.c 0ef0651714728055d43de7a4cdd95e703fac0119
 F src/btmutex.c 9b899c0d8df3bd68f527b0afe03088321b696d3c
-F src/btree.c b0ac995593edf809962b16b7bfa55455c2f31545
+F src/btree.c f8636b7a6fdd0fef8087bbc7dcd64962e40af691
 F src/btree.h f70b694e8c163227369a66863b01fbff9009f323
 F src/btreeInt.h df64030d632f8c8ac217ed52e8b6b3eacacb33a5
 F src/build.c 20e02fd72249159ff6829009f3029d16d59cdff5
@@ -151,7 +151,7 @@ F src/pager.h 73f481a308a873ccd626d97331c081db3b53e2e5
 F src/parse.y 07690df997d50b3fdb5e5121e5a27f1a080db13d
 F src/pcache.c 395f752a13574120bd7513a400ba02a265aaa76d
 F src/pcache.h 9b927ccc5a538e31b4c3bc7eec4f976db42a1324
-F src/pcache1.c bed75f157283e7c666f323df0c874c6a2515f76e
+F src/pcache1.c 3de4feb556a11a62febe172ca98655dff68a0df3
 F src/pragma.c 06b3a4b93a5e587f1c04b4a40016eb360792cdf3
 F src/prepare.c c98c1d306ef72dd448ecbc3c52624439c72ad413
 F src/printf.c 508a1c59433353552b6553cba175eaa7331f8fc1
@@ -733,7 +733,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff
 F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224
 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e
 F tool/vdbe-compress.tcl 672f81d693a03f80f5ae60bfefacd8a349e76746
-P 6dbf4eca00f845baa7200aba421d0bc158ba96aa
-R 082e50f4fb6f4bbdc7df5b4799b07891
-U danielk1977
-Z f49134d2c7bfd4d13daac32bbf8ae5be
+P 7fa5d3cb0fa05f7d901bcc139c2c037ce5944caa
+R 239128eec97f3bd718312f4ed9a40574
+U drh
+Z 51f7f97ff293b0e414ff2ed7ce212f49

diff --git a/manifest.uuid b/manifest.uuid
index f542f718cefbb5c6584497a0eab71e1a3c50e407..fc87be67a99ea008a5a1be1652b848d165bf0fbc 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-7fa5d3cb0fa05f7d901bcc139c2c037ce5944caa
\ No newline at end of file
+c54de1f54080de7e134d7b562498abb5337a0a46
\ No newline at end of file

diff --git a/src/btree.c b/src/btree.c
index 6309d031afea47e8218aff55d8f112ac49e71bb0..d0285aa1b205a98f6af9ed075ceb34a378dca83e 100644 (file)
--- 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.611 2009/06/03 17:26:18 danielk1977 Exp $
+** $Id: btree.c,v 1.612 2009/06/03 21:04:36 drh Exp $
 **
 ** This file implements a external (disk-based) database using BTrees.
 ** See the header comment on "btreeInt.h" for additional information.
@@ -1142,7 +1142,38 @@ int sqlite3BtreeInitPage(MemPage *pPage){
       /* To many cells for a single page.  The page must be corrupt */
       return SQLITE_CORRUPT_BKPT;
     }
-  
+
+    /* A malformed database page might cause use to read past the end
+    ** of page when parsing a cell.  
+    **
+    ** The following block of code checks early to see if a cell extends
+    ** past the end of a page boundary and causes SQLITE_CORRUPT to be 
+    ** returned if it does.
+    */
+#if defined(SQLITE_OVERREAD_CHECK) || 1
+    {
+      int iCellFirst;   /* First allowable cell index */
+      int iCellLast;    /* Last possible cell index */
+      int i;            /* Index into the cell pointer array */
+      int sz;           /* Size of a cell */
+
+      iCellFirst = cellOffset + 2*pPage->nCell;
+      iCellLast = usableSize - 4;
+      if( !pPage->leaf ) iCellLast--;
+      for(i=0; i<pPage->nCell; i++){
+        pc = get2byte(&data[cellOffset+i*2]);
+        if( pc<iCellFirst || pc>iCellLast ){
+          return SQLITE_CORRUPT_BKPT;
+        }
+        sz = cellSizePtr(pPage, &data[pc]);
+        if( pc+sz>usableSize ){
+          return SQLITE_CORRUPT_BKPT;
+        }
+      }
+    }  
+#endif
+
+
     /* Compute the total free space on the page */
     pc = get2byte(&data[hdr+1]);
     nFree = data[hdr+7] + top;

diff --git a/src/pcache1.c b/src/pcache1.c
index b41bb6732598fe19508756fa1aa70f5c1ad375f1..879705ff62d67b1d2bb2c8c4da2d3705274fdf4d 100644 (file)
--- a/src/pcache1.c
+++ b/src/pcache1.c
@@ -16,7 +16,7 @@
 ** If the default page cache implementation is overriden, then neither of
 ** these two features are available.
 **
-** @(#) $Id: pcache1.c,v 1.15 2009/05/22 11:12:23 drh Exp $
+** @(#) $Id: pcache1.c,v 1.16 2009/06/03 21:04:36 drh Exp $
 */
 
 #include "sqliteInt.h"
@@ -54,7 +54,7 @@ struct PCache1 {
 /*
 ** Each cache entry is represented by an instance of the following 
 ** structure. A buffer of PgHdr1.pCache->szPage bytes is allocated 
-** directly after the structure in memory (see the PGHDR1_TO_PAGE() 
+** directly before this structure in memory (see the PGHDR1_TO_PAGE() 
 ** macro below).
 */
 struct PgHdr1 {
@@ -100,7 +100,7 @@ static SQLITE_WSD struct PCacheGlobal {
 
 /*
 ** When a PgHdr1 structure is allocated, the associated PCache1.szPage
-** bytes of data are located directly after it in memory (i.e. the total
+** bytes of data are located directly before it in memory (i.e. the total
 ** size of the allocation is sizeof(PgHdr1)+PCache1.szPage byte). The
 ** PGHDR1_TO_PAGE() macro takes a pointer to a PgHdr1 structure as
 ** an argument and returns a pointer to the associated block of szPage
@@ -108,10 +108,10 @@ static SQLITE_WSD struct PCacheGlobal {
 ** a pointer to a block of szPage bytes of data and the return value is
 ** a pointer to the associated PgHdr1 structure.
 **
-**   assert( PGHDR1_TO_PAGE(PAGE_TO_PGHDR1(X))==X );
+**   assert( PGHDR1_TO_PAGE(PAGE_TO_PGHDR1(pCache, X))==X );
 */
-#define PGHDR1_TO_PAGE(p) (void *)(&((unsigned char *)p)[sizeof(PgHdr1)])
-#define PAGE_TO_PGHDR1(p) (PgHdr1 *)(&((unsigned char *)p)[-1*(int)sizeof(PgHdr1)])
+#define PGHDR1_TO_PAGE(p)    (void*)(((char*)p) - p->pCache->szPage)
+#define PAGE_TO_PGHDR1(c, p) (PgHdr1*)(((char*)p) + c->szPage)
 
 /*
 ** Macros to enter and leave the global LRU mutex.
@@ -203,11 +203,15 @@ static void pcache1Free(void *p){
 */
 static PgHdr1 *pcache1AllocPage(PCache1 *pCache){
   int nByte = sizeof(PgHdr1) + pCache->szPage;
-  PgHdr1 *p = (PgHdr1 *)pcache1Alloc(nByte);
-  if( p ){
+  void *pPg = pcache1Alloc(nByte);
+  PgHdr1 *p;
+  if( pPg ){
+    p = PAGE_TO_PGHDR1(pCache, pPg);
     if( pCache->bPurgeable ){
       pcache1.nCurrentPage++;
     }
+  }else{
+    p = 0;
   }
   return p;
 }
@@ -220,7 +224,7 @@ static void pcache1FreePage(PgHdr1 *p){
     if( p->pCache->bPurgeable ){
       pcache1.nCurrentPage--;
     }
-    pcache1Free(p);
+    pcache1Free(PGHDR1_TO_PAGE(p));
   }
 }
 
@@ -560,13 +564,13 @@ static void *pcache1Fetch(sqlite3_pcache *p, unsigned int iKey, int createFlag){
 
   if( pPage ){
     unsigned int h = iKey % pCache->nHash;
-    *(void **)(PGHDR1_TO_PAGE(pPage)) = 0;
     pCache->nPage++;
     pPage->iKey = iKey;
     pPage->pNext = pCache->apHash[h];
     pPage->pCache = pCache;
     pPage->pLruPrev = 0;
     pPage->pLruNext = 0;
+    *(void **)(PGHDR1_TO_PAGE(pPage)) = 0;
     pCache->apHash[h] = pPage;
   }
 
@@ -587,8 +591,9 @@ fetch_out:
 */
 static void pcache1Unpin(sqlite3_pcache *p, void *pPg, int reuseUnlikely){
   PCache1 *pCache = (PCache1 *)p;
-  PgHdr1 *pPage = PAGE_TO_PGHDR1(pPg);
-
+  PgHdr1 *pPage = PAGE_TO_PGHDR1(pCache, pPg);
+ 
+  assert( pPage->pCache==pCache );
   pcache1EnterMutex();
 
   /* It is an error to call this function if the page is already 
@@ -630,10 +635,11 @@ static void pcache1Rekey(
   unsigned int iNew
 ){
   PCache1 *pCache = (PCache1 *)p;
-  PgHdr1 *pPage = PAGE_TO_PGHDR1(pPg);
+  PgHdr1 *pPage = PAGE_TO_PGHDR1(pCache, pPg);
   PgHdr1 **pp;
   unsigned int h; 
   assert( pPage->iKey==iOld );
+  assert( pPage->pCache==pCache );
 
   pcache1EnterMutex();
 
@@ -728,7 +734,7 @@ int sqlite3PcacheReleaseMemory(int nReq){
     PgHdr1 *p;
     pcache1EnterMutex();
     while( (nReq<0 || nFree<nReq) && (p=pcache1.pLruTail) ){
-      nFree += sqlite3MallocSize(p);
+      nFree += sqlite3MallocSize(PGHDR1_TO_PAGE(p));
       pcache1PinPage(p);
       pcache1RemoveFromHash(p);
       pcache1FreePage(p);