Skip to content

Fix three nested-VFS defects: FullPathname off-by-one, urlencode stack overflow, Close() leak on Sync failure - #18

Merged
mlin merged 1 commit into
mlin:mainfrom
Wid-L-Hacker:conrad/vfs-audit-fixes
Oct 24, 2026
Merged

Fix three nested-VFS defects: FullPathname off-by-one, urlencode stack overflow, Close() leak on Sync failure#18
mlin merged 1 commit into
mlin:mainfrom
Wid-L-Hacker:conrad/vfs-audit-fixes

Conversation

@Wid-L-Hacker

@Wid-L-Hacker Wid-L-Hacker commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Three defects found by audit (all reproduced under AddressSanitizer before the fix, clean after), each a one-line fix in src/SQLiteNestedVFS.h:

  1. FullPathname off-by-one — the terminating NUL was written at zPathOut[nPathOut], one byte past the buffer SQLite provided.
  2. urlencode stack overflow — a filename byte >= 0x80, passed as a signed char to sprintf("%.2X", ...), printed as FFFFFFxx: 9 bytes into a 3-byte stack buffer. Any non-ASCII filename (e.g. café.db) hit it on open under x86-64 Linux.
  3. Close() leak on Sync failure — if the final Sync failed, InnerDatabaseFile::Close returned the error without self-deleting. The leaked object kept its outer database connection (locking_mode=EXCLUSIVE) alive, leaving the outer database write-locked for the rest of the process's life. The exception path in the same function already self-deleted; the Sync path now does the same.

Severity P1 on all three (edge-case triggers). Full analysis: ops record in the audit notes.

@Wid-L-Hacker
Fix three nested-VFS defects found in audit
11d9edb 
1. FullPathname: terminate zPathOut at nPathOut-1, not nPathOut
   (one byte past the caller's buffer on every call).
2. urlencode: format the byte as unsigned char; a signed char >= 0x80
   printed as FFFFFFxx, overrunning the 3-byte stack buffer on any
   non-ASCII filename (x86-64 Linux).
3. InnerDatabaseFile::Close: when the final Sync fails, still
   self-delete via SQLiteVFS::File::Close() before returning the
   error (as the exception path already does). The early return
   leaked the file object and its outer database connection, which
   holds locking_mode=EXCLUSIVE, leaving the outer database
   write-locked by a ghost for the process's remaining life.

All three reproduced under AddressSanitizer before the fix and
clean after (harness: exact-size FullPathname buffer, UTF-8
filename open, forced close-time Sync failure with lock probe).
@mlin
mlin merged commit 6e5a177 into mlin:main Oct 24, 2026
5 checks passed
@mlin
mlin deleted the conrad/vfs-audit-fixes branch October 24, 2026 04:36
Wid-L-Hacker added a commit to Wid-L-Hacker/sqlite_zstd_vfs that referenced this pull request Oct 25, 2026
@Wid-L-Hacker
Advance sqlite_web_vfs pin to 176801622
2f24a1e 
Brings the web fixes merged since 16072b1 into this build: F5 +
ThreadPool C1/C2 (mlin#17), F6 Close self-delete (mlin#18), F7 Content-Range
validation (mlin#19), C6 last_error_ synchronization (mlin#20).
@Wid-L-Hacker Wid-L-Hacker mentioned this pull request Oct 25, 2026
Merged
mlin pushed a commit that referenced this pull request Oct 25, 2026
@Wid-L-Hacker
Advance sqlite_web_vfs pin to 176801622 (#21)
251d7b0 
Brings the web fixes merged since 16072b1 into this build: F5 +
ThreadPool C1/C2 (#17), F6 Close self-delete (#18), F7 Content-Range
validation (#19), C6 last_error_ synchronization (#20).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Wid-L-Hacker @mlin