From: dan Date: Wed, 10 Jun 2026 20:10:19 +0000 (+0000) Subject: When reading a super-journal name from a journal file, allocate a new buffer rather... X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=dacabb4a33bb12052ad3c40dea97ea828588a97c;p=thirdparty%2Fsqlite.git 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: 37aa82a50a0a1c5edb4c4f2de1cb00fa009ebbfeb420df3de72231c5d883518a --- diff --git a/manifest b/manifest index 2778e29be6..b112d0c8c3 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\sa\ssigned-integer\soverflow\sin\sfts5\sthat\smight\soccur\swhen\sdealing\swith\sstrategicly\scorrupted\srecords.\sBug\s[bugs:/info/2026-06-10T03:56:42Z\s|\s2026-06-10T03:56:42Z]. -D 2026-06-10T16:51:20.171 +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-10T20:10:19.569 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 8df4b34ec6a08616a7ac33164999524ef773fa359d39ae9ae0e7e1ae4f167440 F src/os_win.h c06ccc3a090cf54202ea58981c298817f3309d4c9e4d52ad0a02927346493721 -F src/pager.c e0b3b6e39c3a783957d640b28401401d1f3c556803c80695958dd2b9db4ef72d +F src/pager.c cb691a81605f211d88d945e8d0ead84cf654ac7b19245cd65b3e5e69ea87e024 F src/pager.h 6137149346e6c8a3ddc1eeb40aee46381e9bc8b0fcc6dda8a1efde993c2275b8 F src/parse.y d5a3c5b0277a441c38b35071c05e2b61ff5fc918a63309c809f4b6706179c320 F src/pcache.c 588cc3c5ccaaadde689ed35ce5c5c891a1f7b1f4d1f56f6cf0143b74d8ee6484 @@ -2209,8 +2209,11 @@ F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee F tool/warnings.sh a554d13f6e5cf3760f041b87939e3d616ec6961859c3245e8ef701d1eafc2ca2 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f F tool/winmain.c 00c8fb88e365c9017db14c73d3c78af62194d9644feaf60e220ab0f411f3604c -P 8b961dc3d27c5aa62a5dc7c2e44f8b505817e184f8499f3bb903e06b5aec1b72 -R 1f6a43a747295a1933a4aa3b45b558bb +P fc6442ee54795fbeb746539193716238aa653d80170523bc327ae3ce0d945ebf +R d78d4f2bafad585e32d486b41a8f1c32 +T *branch * super-journal-malloc +T *sym-super-journal-malloc * +T -sym-trunk * U dan -Z d194393ed734fcf872689abff265089a +Z 2d91bb66833487307406c342f82ea2df # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.tags b/manifest.tags index bec971799f..8cb29a232b 100644 --- a/manifest.tags +++ b/manifest.tags @@ -1,2 +1,2 @@ -branch trunk -tag trunk +branch super-journal-malloc +tag super-journal-malloc diff --git a/manifest.uuid b/manifest.uuid index 1d32beea58..72d6a06437 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -fc6442ee54795fbeb746539193716238aa653d80170523bc327ae3ce0d945ebf +37aa82a50a0a1c5edb4c4f2de1cb00fa009ebbfeb420df3de72231c5d883518a diff --git a/src/pager.c b/src/pager.c index f8c38f9d4e..6f5964acc3 100644 --- a/src/pager.c +++ b/src/pager.c @@ -1271,31 +1271,7 @@ static void checkPage(PgHdr *pPg){ #define CHECK_PAGE(x) #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. -*/ +#if 0 static int readSuperJournal(sqlite3_file *pJrnl, char *zSuper, u64 nSuper){ int rc; /* Return code */ u32 len; /* Length in bytes of super-journal name */ @@ -1336,6 +1312,85 @@ static int readSuperJournal(sqlite3_file *pJrnl, char *zSuper, u64 nSuper){ return SQLITE_OK; } +#endif + + +/* +** Free a buffer allocated by the readSuperJournal() function. +*/ +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. +** +** Parmeter 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 */ + unsigned char aMagic[8]; /* A buffer to hold the magic header */ + char *zOut = 0; + + *pzSuper = 0; + if( SQLITE_OK!=(rc = sqlite3OsFileSize(pJrnl, &szJ)) + || szJ<16 + || SQLITE_OK!=(rc = read32bits(pJrnl, szJ-16, &len)) + || len>=nSuper + || len>szJ-16 + || len==0 + || SQLITE_OK!=(rc = read32bits(pJrnl, szJ-12, &cksum)) + || SQLITE_OK!=(rc = sqlite3OsRead(pJrnl, aMagic, 8, szJ-8)) + || memcmp(aMagic, aJournalMagic, 8) + ){ + return rc; + } + + zOut = (char*)sqlite3MallocZero(4 + len + 2); + if( !zOut ){ + rc = SQLITE_NOMEM; + }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; umxPathname; - 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 +2627,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 +2640,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 +2656,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; @@ -2831,12 +2886,10 @@ static int pager_playback(Pager *pPager, int isHot){ ** 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 +3018,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 +3044,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; }