Skip to content

fix: create extracted library and db copy readable by owner only - #1488

Open
sahvx655-wq wants to merge 1 commit into
xerial:masterfrom
sahvx655-wq:temp-file-owner-only
Open

sahvx655-wq wants to merge 1 commit into
xerial:masterfrom
sahvx655-wq:temp-file-owner-only

Conversation

@sahvx655-wq

Copy link
Copy Markdown
Contributor

Both temporary copies the driver writes into the shared temp directory come out with whatever the umask allows. SQLiteJDBCLoader.extractAndLoadLibraryFile goes through Files.copy and SQLiteConnection.extractResource through a plain FileOutputStream, so with the usual 022 the extracted native library is 0755 and the :resource: database copy is 0644, which leaves a database the application then opens read-write readable by every other local user. Running the extraction under umask 002 I get rwxrw-r-- on the shared object and rw-rw-r-- on the database copy, so another user in the group can rewrite the library the JVM has loaded, or edit the data behind the connection. The content check that runs before System.load does not help, the file stays writable for the life of the JVM and nothing re-verifies it.

The library is now created with Files.createTempFile, which is 0600 atomically on POSIX, and the contents are streamed into that file rather than through Files.copy with REPLACE_EXISTING, which deletes and recreates the target with default permissions; the read and execute bits are then set for the owner only. The database copy is pre-created with createNewFile and restricted to the owner before any data is written, using java.io only since that class has to stay Android compatible. Both paths get a regression test asserting the POSIX permissions of the copy, and both fail on the current tree with GROUP_READ and OTHERS_READ present.

@gotson

gotson commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

did you remove the UUID bit in the filename ?

@sahvx655-wq

sahvx655-wq commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor Author

Yes, the explicit UUID is gone. Files.createTempFile puts its own random part where the UUID used to be, an unsigned random long from a SecureRandom, so names now look like sqlite-3.53.4-8412907716302458121-libsqlitejdbc.so. It also creates the file with CREATE_NEW and retries on a collision, so two JVMs sharing the folder still can't end up with the same file. I checked cleanup() too: it only matches on the sqlite-<version> prefix and the .lck sibling, so stale libraries are still found and removed the same way.

If you'd rather keep the UUID in the name for familiarity, I can pass sqlite-<version>-<uuid>- as the prefix. That costs nothing, though you'd get the random suffix as well. One thing I noticed while re-reading this: the library file now exists a moment before its .lck is created, whereas before the lock came first. So in theory another JVM's cleanup() could delete the empty file in that gap, and newOutputStream would then recreate it with default permissions. It's a narrow window, but I'm happy to tighten it in the same follow-up. That would mean going back to a UUID name I choose up front, creating the .lck first, then creating the library with CREATE_NEW and owner-only permissions, since createTempFile only reveals the name once the file already exists.

@gotson

gotson commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the explanation, we are not necessarily attached to the UUID, but tightening the race condition would be welcome.

I'm wondering if we should try to tackle #1140 at the same time though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants