From 3cdade9ad7c1414ba0bd2b6dc98fdb8ed44f3311 Mon Sep 17 00:00:00 2001 From: ericek111 Date: Fri, 25 Sep 2026 10:55:09 +0000 Subject: [PATCH] Keep the profile private and write it all at once Settings, bookmarks (server passwords) and identities (private keys) were written in place with the default permissions, readable by every user on the machine, and a crash mid-write left a truncated file that the next start silently replaced with defaults. Every profile file is now written to a private temporary file, synced and moved over the old one in one step. The profile folder itself is made accessible to its owner only, which also covers files written before. Co-Authored-By: Claude Opus 5.5 --- .../java/com/ts3client/config/AppDirs.java | 9 ++- .../com/ts3client/config/AwayMessages.java | 5 +- .../ts3client/config/BanReasonPresets.java | 5 +- .../java/com/ts3client/config/Bookmarks.java | 5 +- .../com/ts3client/config/IdentityStore.java | 2 +- .../com/ts3client/config/ProfileFiles.java | 77 +++++++++++++++++++ .../java/com/ts3client/config/Settings.java | 5 +- .../com/ts3client/contacts/ContactStore.java | 3 +- .../java/com/ts3client/hotkey/Hotkeys.java | 6 +- .../ts3client/config/ProfileFilesTest.java | 49 ++++++++++++ 10 files changed, 142 insertions(+), 24 deletions(-) create mode 100644 ts3-client/core/src/main/java/com/ts3client/config/ProfileFiles.java create mode 100644 ts3-client/core/src/test/java/com/ts3client/config/ProfileFilesTest.java diff --git a/ts3-client/core/src/main/java/com/ts3client/config/AppDirs.java b/ts3-client/core/src/main/java/com/ts3client/config/AppDirs.java index 6fa8d7b..94ef603 100644 --- a/ts3-client/core/src/main/java/com/ts3client/config/AppDirs.java +++ b/ts3-client/core/src/main/java/com/ts3client/config/AppDirs.java @@ -36,10 +36,15 @@ public final class AppDirs { return new File(profile(), name); } - /** Creates the profile directory if it does not exist yet; call before writing into it. */ + /** + * Creates the profile directory if it does not exist yet, readable by the user only; call + * before writing into it. + */ public static void createProfile() { + File dir = profile(); //noinspection ResultOfMethodCallIgnored - profile().mkdirs(); + dir.mkdirs(); + ProfileFiles.makePrivate(dir); } public static Path chatLogs() { diff --git a/ts3-client/core/src/main/java/com/ts3client/config/AwayMessages.java b/ts3-client/core/src/main/java/com/ts3client/config/AwayMessages.java index dbf4b50..fd50b0d 100644 --- a/ts3-client/core/src/main/java/com/ts3client/config/AwayMessages.java +++ b/ts3-client/core/src/main/java/com/ts3client/config/AwayMessages.java @@ -2,7 +2,6 @@ package com.ts3client.config; import java.io.File; import java.io.FileInputStream; -import java.io.FileOutputStream; import java.util.ArrayList; import java.util.List; import java.util.Properties; @@ -60,9 +59,7 @@ public final class AwayMessages { } try { AppDirs.createProfile(); - try (FileOutputStream out = new FileOutputStream(file())) { - p.store(out, "TS3J client away messages"); - } + ProfileFiles.write(file(), out -> p.store(out, "TS3J client away messages")); } catch (Exception ignored) { } } diff --git a/ts3-client/core/src/main/java/com/ts3client/config/BanReasonPresets.java b/ts3-client/core/src/main/java/com/ts3client/config/BanReasonPresets.java index a690cc7..d67b6e7 100644 --- a/ts3-client/core/src/main/java/com/ts3client/config/BanReasonPresets.java +++ b/ts3-client/core/src/main/java/com/ts3client/config/BanReasonPresets.java @@ -2,7 +2,6 @@ package com.ts3client.config; import java.io.File; import java.io.FileInputStream; -import java.io.FileOutputStream; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -60,9 +59,7 @@ public final class BanReasonPresets { } try { AppDirs.createProfile(); - try (FileOutputStream out = new FileOutputStream(file())) { - p.store(out, "TS3J client ban reason presets"); - } + ProfileFiles.write(file(), out -> p.store(out, "TS3J client ban reason presets")); } catch (Exception ignored) { } } diff --git a/ts3-client/core/src/main/java/com/ts3client/config/Bookmarks.java b/ts3-client/core/src/main/java/com/ts3client/config/Bookmarks.java index 6b2abff..d0d8613 100644 --- a/ts3-client/core/src/main/java/com/ts3client/config/Bookmarks.java +++ b/ts3-client/core/src/main/java/com/ts3client/config/Bookmarks.java @@ -2,7 +2,6 @@ package com.ts3client.config; import java.io.File; import java.io.FileInputStream; -import java.io.FileOutputStream; import java.util.ArrayList; import java.util.List; import java.util.Properties; @@ -85,9 +84,7 @@ public final class Bookmarks { } try { AppDirs.createProfile(); - try (FileOutputStream out = new FileOutputStream(file())) { - p.store(out, "TS3J client bookmarks"); - } + ProfileFiles.write(file(), out -> p.store(out, "TS3J client bookmarks")); } catch (Exception ignored) { } } diff --git a/ts3-client/core/src/main/java/com/ts3client/config/IdentityStore.java b/ts3-client/core/src/main/java/com/ts3client/config/IdentityStore.java index fb94777..deb0c99 100644 --- a/ts3-client/core/src/main/java/com/ts3client/config/IdentityStore.java +++ b/ts3-client/core/src/main/java/com/ts3client/config/IdentityStore.java @@ -231,7 +231,7 @@ public final class IdentityStore { } Map props = new HashMap<>(); props.put("id", name == null ? "" : name); - identity.save(file, props); + ProfileFiles.write(file, out -> identity.save(out, props)); } private static IdentityEntry read(File file) { diff --git a/ts3-client/core/src/main/java/com/ts3client/config/ProfileFiles.java b/ts3-client/core/src/main/java/com/ts3client/config/ProfileFiles.java new file mode 100644 index 0000000..8ecae80 --- /dev/null +++ b/ts3-client/core/src/main/java/com/ts3client/config/ProfileFiles.java @@ -0,0 +1,77 @@ +package com.ts3client.config; + +import java.io.File; +import java.io.FilterOutputStream; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.channels.Channels; +import java.nio.channels.FileChannel; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.PosixFilePermissions; + +/** + * Writes the files in the profile, which hold private keys and server passwords: only the + * user may read them, and a crash mid-write never leaves one cut short. + */ +public final class ProfileFiles { + + /** Produces a file's content. */ + public interface Content { + void writeTo(OutputStream out) throws IOException; + } + + private ProfileFiles() { + } + + /** + * Replaces {@code file} with {@code content} in one step: it is written to a private + * temporary file next to it, flushed to disk, then moved over the old one. + */ + public static void write(File file, Content content) throws IOException { + Path target = file.toPath().toAbsolutePath(); + Path dir = target.getParent(); + Files.createDirectories(dir); + // Created readable by the owner only, where the file system has permissions at all. + Path temp = Files.createTempFile(dir, "." + file.getName() + ".", ".tmp"); + try { + try (FileChannel channel = FileChannel.open(temp, StandardOpenOption.WRITE)) { + content.writeTo(unclosable(Channels.newOutputStream(channel))); + channel.force(true); + } + try { + Files.move(temp, target, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); + } catch (AtomicMoveNotSupportedException e) { + Files.move(temp, target, StandardCopyOption.REPLACE_EXISTING); + } + } finally { + Files.deleteIfExists(temp); + } + } + + /** Some writers close what they are given; the file still has to be synced after them. */ + private static OutputStream unclosable(OutputStream out) { + return new FilterOutputStream(out) { + @Override + public void write(byte[] b, int off, int len) throws IOException { + out.write(b, off, len); + } + + @Override + public void close() { + } + }; + } + + /** Makes a directory accessible to the user only, where the file system supports that. */ + static void makePrivate(File dir) { + try { + Files.setPosixFilePermissions(dir.toPath(), PosixFilePermissions.fromString("rwx------")); + } catch (UnsupportedOperationException | IOException ignored) { + // Not a POSIX file system (Windows): the user's profile folder is private already. + } + } +} diff --git a/ts3-client/core/src/main/java/com/ts3client/config/Settings.java b/ts3-client/core/src/main/java/com/ts3client/config/Settings.java index 65ed654..42e2742 100644 --- a/ts3-client/core/src/main/java/com/ts3client/config/Settings.java +++ b/ts3-client/core/src/main/java/com/ts3client/config/Settings.java @@ -5,7 +5,6 @@ import com.ts3client.sound.NotificationSettings; import java.io.File; import java.io.FileInputStream; -import java.io.FileOutputStream; import java.util.Properties; /** @@ -207,9 +206,7 @@ public final class Settings { try { AppDirs.createProfile(); writeToProps(); - try (FileOutputStream out = new FileOutputStream(file())) { - props.store(out, "TS3J Swing Client settings"); - } + ProfileFiles.write(file(), out -> props.store(out, "TS3J Swing Client settings")); } catch (Exception ignored) { } } diff --git a/ts3-client/core/src/main/java/com/ts3client/contacts/ContactStore.java b/ts3-client/core/src/main/java/com/ts3client/contacts/ContactStore.java index 169ea5b..8a66c65 100644 --- a/ts3-client/core/src/main/java/com/ts3client/contacts/ContactStore.java +++ b/ts3-client/core/src/main/java/com/ts3client/contacts/ContactStore.java @@ -1,6 +1,7 @@ package com.ts3client.contacts; import com.ts3client.config.AppDirs; +import com.ts3client.config.ProfileFiles; import com.ts3client.teamspeak.TeamSpeakSettingsDb; import java.io.File; @@ -94,7 +95,7 @@ public final class ContactStore { } try { AppDirs.createProfile(); - Files.writeString(file.toPath(), sb.toString(), StandardCharsets.UTF_8); + ProfileFiles.write(file, out -> out.write(sb.toString().getBytes(StandardCharsets.UTF_8))); } catch (IOException ignored) { } } diff --git a/ts3-client/core/src/main/java/com/ts3client/hotkey/Hotkeys.java b/ts3-client/core/src/main/java/com/ts3client/hotkey/Hotkeys.java index 1948ace..31ca3c5 100644 --- a/ts3-client/core/src/main/java/com/ts3client/hotkey/Hotkeys.java +++ b/ts3-client/core/src/main/java/com/ts3client/hotkey/Hotkeys.java @@ -1,10 +1,10 @@ package com.ts3client.hotkey; import com.ts3client.config.AppDirs; +import com.ts3client.config.ProfileFiles; import java.io.File; import java.io.FileInputStream; -import java.io.FileOutputStream; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -93,9 +93,7 @@ public final class Hotkeys { } try { AppDirs.createProfile(); - try (FileOutputStream out = new FileOutputStream(file())) { - p.store(out, "TS3J client hotkeys"); - } + ProfileFiles.write(file(), out -> p.store(out, "TS3J client hotkeys")); } catch (Exception ignored) { } } diff --git a/ts3-client/core/src/test/java/com/ts3client/config/ProfileFilesTest.java b/ts3-client/core/src/test/java/com/ts3client/config/ProfileFilesTest.java new file mode 100644 index 0000000..f32d881 --- /dev/null +++ b/ts3-client/core/src/test/java/com/ts3client/config/ProfileFilesTest.java @@ -0,0 +1,49 @@ +package com.ts3client.config; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermissions; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class ProfileFilesTest { + + @TempDir + Path dir; + + @Test + void replacesTheFileReadableByTheUserOnly() throws Exception { + File file = dir.resolve("settings.properties").toFile(); + Files.writeString(file.toPath(), "old"); + + ProfileFiles.write(file, out -> out.write("new".getBytes())); + + assertEquals("new", Files.readString(file.toPath())); + assertEquals("rw-------", PosixFilePermissions.toString(Files.getPosixFilePermissions(file.toPath()))); + try (var left = Files.list(dir)) { + assertEquals(1, left.count(), "no temporary file left behind"); + } + } + + @Test + void aFailedWriteLeavesTheOldFileAlone() throws Exception { + File file = dir.resolve("bookmarks.properties").toFile(); + Files.writeString(file.toPath(), "old"); + + assertThrows(IOException.class, () -> ProfileFiles.write(file, out -> { + out.write("half".getBytes()); + throw new IOException("crashed"); + })); + + assertEquals("old", Files.readString(file.toPath())); + try (var left = Files.list(dir)) { + assertEquals(1, left.count(), "no temporary file left behind"); + } + } +}