When reading a super-journal name from a journal file, allocate a new buffer rather than using Pager.pTmpSpace. This prevents a buffer overrun that could occur when using a VFS with a large sqlite3_vfs.mxPathname value with a database with a small page size.

FossilOrigin-Name: 7f71859841af7cb0806f58e9c8013a990fcca72b807a0513156d7127ce5c7b62
2026-06-12 18:35:24 +00:00
parent 2ca10782d2 7459c01689
commit 2884421c05
3 changed files with 77 additions and 82 deletions
@@ -1,5 +1,5 @@
-C Improved\sdocumentation\sfor\ssqlite3_deserialize().
-D 2026-06-12T16:47:45.833
+C When\sreading\sa\ssuper-journal\sname\sfrom\sa\sjournal\sfile,\sallocate\sa\snew\sbuffer\srather\sthan\susing\sPager.pTmpSpace.\sThis\sprevents\sa\sbuffer\soverrun\sthat\scould\soccur\swhen\susing\sa\sVFS\swith\sa\slarge\ssqlite3_vfs.mxPathname\svalue\swith\sa\sdatabase\swith\sa\ssmall\spage\ssize.
+D 2026-06-12T18:35:24.464
 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
@@ -726,7 +726,7 @@ F src/os_setup.h 8efc64eda6a6c2f221387eefc2e7e45fd5a3d5c8337a7a83519ba4fbd2957ae
 F src/os_unix.c 83759942d1ea8d59daed50901c123016f845fada74caf3496b8a2537c9a08838
 F src/os_win.c 68b1c31693a5aeeb8126f618c95f7b53fb39e254836f9a95fbf2733461a7e01d
 F src/os_win.h c06ccc3a090cf54202ea58981c298817f3309d4c9e4d52ad0a02927346493721
-F src/pager.c e0b3b6e39c3a783957d640b28401401d1f3c556803c80695958dd2b9db4ef72d
+F src/pager.c f88073a00933c885b167b6d25afc4d1b83c1706943572f5653fb64a7f5bde105
 F src/pager.h 6137149346e6c8a3ddc1eeb40aee46381e9bc8b0fcc6dda8a1efde993c2275b8
 F src/parse.y d5a3c5b0277a441c38b35071c05e2b61ff5fc918a63309c809f4b6706179c320
 F src/pcache.c 588cc3c5ccaaadde689ed35ce5c5c891a1f7b1f4d1f56f6cf0143b74d8ee6484
@@ -2209,8 +2209,9 @@ F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee
 F tool/warnings.sh a554d13f6e5cf3760f041b87939e3d616ec6961859c3245e8ef701d1eafc2ca2
 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f
 F tool/winmain.c 00c8fb88e365c9017db14c73d3c78af62194d9644feaf60e220ab0f411f3604c
-P a4691489f21bb1645bb8de649268dd457af53c5f613f2a7fd2e5569f029efa87
-R 004a022439a0f6ee88072759b334ba56
-U drh
-Z 3666e180ad4544fe7a3a870a71a92f1d
+P f15d076820f4a0d9e7fa32d99575bce27bde5e987bb415e6cfb69b06f0da7e1e ac17669e840dbac5c7c75e9b6014cca2d289aba8de36f9d1d0d61186e8cc6207
+R b4784016cdc66719536104e64f4ddb51
+T +closed ac17669e840dbac5c7c75e9b6014cca2d289aba8de36f9d1d0d61186e8cc6207
+U dan
+Z ae30c5c61ab231f09fb1164a547a0499
 # Remove this line to create a well-formed Fossil manifest.
@@ -1 +1 @@
-f15d076820f4a0d9e7fa32d99575bce27bde5e987bb415e6cfb69b06f0da7e1e
+7f71859841af7cb0806f58e9c8013a990fcca72b807a0513156d7127ce5c7b62
@@ -1272,39 +1272,43 @@ static void checkPage(PgHdr *pPg){
 #endif  /* SQLITE_CHECK_PAGES */

 /*
-** When this is called the journal file for pager pPager must be open.
-** This function attempts to read a super-journal file name from the
-** end of the file and, if successful, copies it into memory supplied
-** by the caller. See comments above writeSuperJournal() for the format
-** used to store a super-journal file name at the end of a journal file.
-**
-** zSuper must point to a buffer of at least nSuper bytes allocated by
-** the caller. This should be sqlite3_vfs.mxPathname+1 (to ensure there is
-** enough space to write the super-journal name). If the super-journal
-** name in the journal is longer than nSuper bytes (including a
-** nul-terminator), then this is handled as if no super-journal name
-** were present in the journal.
-**
-** If a super-journal file name is present at the end of the journal
-** file, then it is copied into the buffer pointed to by zSuper. A
-** nul-terminator byte is appended to the buffer following the
-** super-journal file name.
-**
-** If it is determined that no super-journal file name is present
-** zSuper[0] is set to 0 and SQLITE_OK returned.
-**
-** If an error occurs while reading from the journal file, an SQLite
-** error code is returned.
+** Free a buffer allocated by the readSuperJournal() function.
 */
-static int readSuperJournal(sqlite3_file *pJrnl, char *zSuper, u64 nSuper){
+static void freeSuperJournal(char *zSuper){
+  if( zSuper ){
+    sqlite3_free(&zSuper[-4]);
+  }
+}
+
+/*
+** Parameter pJrnl is a file-handle open on a journal file. This function
+** attempts to read a super-journal file name from the end of the journal 
+** file. If successful, it sets output parameter (*pzSuper) to point to a
+** buffer containing the super-journal name as a nul-terminated string.
+** The caller is responsible for freeing the buffer using freeSuperJournal().
+**
+** Refer to comments above writeSuperJournal() for the format used to store 
+** a super-journal file name at the end of a journal file.
+**
+** Parameter nSuper is passed the maximum allowable size of the super journal
+** name in bytes. If the super-journal name in the journal is longer than
+** nSuper bytes (including a nul-terminator), then this is handled as if no
+** super-journal name were present in the journal.
+**
+** If there is no super-journal name at the end of pJrnl, (*pzSuper) is
+** set to 0 and SQLITE_OK is returned. Or, if an error occurs while reading
+** the super-journal name, an SQLite error code is returned and (*pzSuper)
+** is set to 0.
+*/
+static int readSuperJournal(sqlite3_file *pJrnl, u64 nSuper, char **pzSuper){
  int rc;                    /* Return code */
  u32 len;                   /* Length in bytes of super-journal name */
  i64 szJ;                   /* Total size in bytes of journal file pJrnl */
  u32 cksum;                 /* MJ checksum value read from journal */
-  u32 u;                     /* Unsigned loop counter */
  unsigned char aMagic[8];   /* A buffer to hold the magic header */
-  zSuper[0] = '\0';
+  char *zOut = 0;

+  *pzSuper = 0;
  if( SQLITE_OK!=(rc = sqlite3OsFileSize(pJrnl, &szJ))
   || szJ<16
   || SQLITE_OK!=(rc = read32bits(pJrnl, szJ-16, &len))
@@ -1314,27 +1318,34 @@ static int readSuperJournal(sqlite3_file *pJrnl, char *zSuper, u64 nSuper){
   || SQLITE_OK!=(rc = read32bits(pJrnl, szJ-12, &cksum))
   || SQLITE_OK!=(rc = sqlite3OsRead(pJrnl, aMagic, 8, szJ-8))
   || memcmp(aMagic, aJournalMagic, 8)
-   || SQLITE_OK!=(rc = sqlite3OsRead(pJrnl, zSuper, len, szJ-16-len))
  ){
    return rc;
  }

-  /* See if the checksum matches the super-journal name */
-  for(u=0; u<len; u++){
-    cksum -= zSuper[u];
+  zOut = (char*)sqlite3MallocZero(4 + len + 2);
+  if( !zOut ){
+    rc = SQLITE_NOMEM_BKPT;
+  }else{
+    zOut = &zOut[4];
+    if( SQLITE_OK==(rc = sqlite3OsRead(pJrnl, zOut, len, szJ-16-len)) ){
+      u32 u;                   /* Unsigned loop counter */
+      /* See if the checksum matches the super-journal name */
+      for(u=0; u<len; u++){
+        cksum -= zOut[u];
+      }
+    }
+    if( rc!=SQLITE_OK || cksum ){
+      /* If the checksum doesn't add up, then one or more of the disk sectors
+      ** containing the super-journal filename is corrupted. This means
+      ** definitely roll back, so just return SQLITE_OK and report a (nul)
+      ** super-journal filename.  */
+      freeSuperJournal(zOut);
+      zOut = 0;
+    }
  }
-  if( cksum ){
-    /* If the checksum doesn't add up, then one or more of the disk sectors
-    ** containing the super-journal filename is corrupted. This means
-    ** definitely roll back, so just return SQLITE_OK and report a (nul)
-    ** super-journal filename.
-    */
-    len = 0;
-  }
-  zSuper[len] = '\0';
-  zSuper[len+1] = '\0';
-  
-  return SQLITE_OK;
+
+  *pzSuper = zOut;
+  return rc;
 }

 /*
@@ -2539,9 +2550,7 @@ static int pager_delsuper(Pager *pPager, const char *zSuper){
  char *zSuperJournal = 0;  /* Contents of super-journal file */
  i64 nSuperJournal;        /* Size of super-journal file */
  char *zJournal;           /* Pointer to one journal within MJ file */
-  char *zSuperPtr;          /* Space to hold super-journal filename */
  char *zFree = 0;          /* Free this buffer */
-  i64 nSuperPtr;            /* Amount of space allocated to zSuperPtr[] */

  /* Allocate space for both the pJournal and pSuper file descriptors.
  ** If successful, open the super-journal file for reading.
@@ -2564,9 +2573,8 @@ static int pager_delsuper(Pager *pPager, const char *zSuper){
  */
  rc = sqlite3OsFileSize(pSuper, &nSuperJournal);
  if( rc!=SQLITE_OK ) goto delsuper_out;
-  nSuperPtr = 1 + (i64)pVfs->mxPathname;
-  assert( nSuperJournal>=0 && nSuperPtr>0 );
-  zFree = sqlite3Malloc(4 + nSuperJournal + 2 + nSuperPtr + 2);
+  assert( nSuperJournal>=0 );
+  zFree = sqlite3Malloc(4 + nSuperJournal + 2);
  if( !zFree ){
    rc = SQLITE_NOMEM_BKPT;
    goto delsuper_out;
@@ -2575,7 +2583,6 @@ static int pager_delsuper(Pager *pPager, const char *zSuper){
  }
  zFree[0] = zFree[1] = zFree[2] = zFree[3] = 0;
  zSuperJournal = &zFree[4];
-  zSuperPtr = &zSuperJournal[nSuperJournal+2];
  rc = sqlite3OsRead(pSuper, zSuperJournal, (int)nSuperJournal, 0);
  if( rc!=SQLITE_OK ) goto delsuper_out;
  zSuperJournal[nSuperJournal] = 0;
@@ -2589,6 +2596,8 @@ static int pager_delsuper(Pager *pPager, const char *zSuper){
      goto delsuper_out;
    }
    if( exists ){
+      char *zSuperPtr = 0;
+
      /* One of the journals pointed to by the super-journal exists.
      ** Open it and check if it points at the super-journal. If
      ** so, return without deleting the super-journal file.
@@ -2603,13 +2612,15 @@ static int pager_delsuper(Pager *pPager, const char *zSuper){
        goto delsuper_out;
      }

-      rc = readSuperJournal(pJournal, zSuperPtr, nSuperPtr);
+      rc = readSuperJournal(pJournal, 1+(u64)pVfs->mxPathname, &zSuperPtr);
      sqlite3OsClose(pJournal);
      if( rc!=SQLITE_OK ){
+        assert( zSuperPtr==0 );
        goto delsuper_out;
      }

-      c = zSuperPtr[0]!=0 && strcmp(zSuperPtr, zSuper)==0;
+      c = zSuperPtr!=0 && strcmp(zSuperPtr, zSuper)==0;
+      freeSuperJournal(zSuperPtr);
      if( c ){
        /* We have a match. Do not delete the super-journal file. */
        goto delsuper_out;
@@ -2824,19 +2835,11 @@ static int pager_playback(Pager *pPager, int isHot){
  ** If a super-journal file name is specified, but the file is not
  ** present on disk, then the journal is not hot and does not need to be
  ** played back.
-  **
-  ** TODO: Technically the following is an error because it assumes that
-  ** buffer Pager.pTmpSpace is (mxPathname+1) bytes or larger. i.e. that
-  ** ((pPager->pageSize+8) >= pPager->pVfs->mxPathname+1). Using os_unix.c,
-  ** mxPathname is 512, which is the same as the minimum allowable value
-  ** for pageSize, and so this assumption holds. But it might not for some
-  ** custom VFS.  */
-  zSuper = pPager->pTmpSpace;
-  rc = readSuperJournal(pPager->jfd, zSuper, 1+(i64)pPager->pVfs->mxPathname);
-  if( rc==SQLITE_OK && zSuper[0] ){
+  */
+  rc = readSuperJournal(pPager->jfd, 1+(i64)pPager->pVfs->mxPathname, &zSuper);
+  if( rc==SQLITE_OK && zSuper ){
    rc = sqlite3OsAccess(pVfs, zSuper, SQLITE_ACCESS_EXISTS, &res);
  }
-  zSuper = 0;
  if( rc!=SQLITE_OK || !res ){
    goto end_playback;
  }
@@ -2965,30 +2968,20 @@ end_playback:
  */
  pPager->changeCountDone = pPager->tempFile;

-  if( rc==SQLITE_OK ){
-    /* Leave 4 bytes of space before the super-journal filename in memory.
-    ** This is because it may end up being passed to sqlite3OsOpen(), in
-    ** which case it requires 4 0x00 bytes in memory immediately before
-    ** the filename. */
-    zSuper = &pPager->pTmpSpace[4];
-    rc = readSuperJournal(pPager->jfd, zSuper, 1+(i64)pPager->pVfs->mxPathname);
-    testcase( rc!=SQLITE_OK );
-  }
  if( rc==SQLITE_OK
   && (pPager->eState>=PAGER_WRITER_DBMOD || pPager->eState==PAGER_OPEN)
  ){
    rc = sqlite3PagerSync(pPager, 0);
  }
  if( rc==SQLITE_OK ){
-    rc = pager_end_transaction(pPager, zSuper[0]!='\0', 0);
+    rc = pager_end_transaction(pPager, zSuper!=0, 0);
    testcase( rc!=SQLITE_OK );
  }
-  if( rc==SQLITE_OK && zSuper[0] && res ){
+  if( rc==SQLITE_OK && zSuper && res ){
    /* If there was a super-journal and this routine will return success,
    ** see if it is possible to delete the super-journal.
    */
-    assert( zSuper==&pPager->pTmpSpace[4] );
-    memset(pPager->pTmpSpace, 0, 4);
+    assert( memcmp(&zSuper[-4], "\0\0\0\0", 4)==0 );
    rc = pager_delsuper(pPager, zSuper);
    testcase( rc!=SQLITE_OK );
  }
@@ -3001,6 +2994,7 @@ end_playback:
  ** back a journal created by a process with a different sector size
  ** value. Reset it to the correct value for this process.
  */
+  freeSuperJournal(zSuper);
  setSectorSize(pPager);
  return rc;
 }