always check and restrict existing wallets and backup directories to owner only permissions
What the developer wrote
Authored by Craig Raw
always check and restrict existing wallets and backup directories to owner only permissions
What changed, and why it matters
This update makes Sparrow Wallet automatically tighten the file permissions on wallet and backup folders so only the computer's owner can read them. Previously, if those folders already existed with looser permissions (for example, created by an older version or another user), other accounts on the same machine might have been able to read wallet files. The change also adds a warning if the app cannot fix the permissions.
Users running Sparrow on shared or multi-user systems should upgrade to the version containing this commit and verify that their wallets/ and wallets/backup/ directories are readable only by their own user (e.g., ls -ld ~/.sparrow/wallets). Review any prior backups stored with broader permissions.
Security signals we found
Permission hardening for sensitive wallet storage directories
New setOwnerOnlyDirectory helper enforcing rwx------ on existing directories
Backup directory permissions now always restricted on startup
Default wallets directory permissions now restricted if already present
Warning logged when permissions cannot be restricted, with deduplication
Evidence from the diff
The commit adds setOwnerOnlyDirectory() in Storage.java. It is called for the default wallets directory and the wallets backup directory when they already exist. On POSIX systems, it reads current POSIX permissions and, if they are not owner-only (rwx------), sets them to owner-only. Symbolic links and Windows are skipped. A ConcurrentHashMap-backed warnedDirectories set prevents duplicate log warnings when Files.setPosixFilePermissions fails. This is a hardening patch rather than a fix for an active exploit path.
Changed components
src/main/java/com/sparrowwallet/sparrow/io/Storage.javaWallet storage directory permissionsWallet backup directory permissionsInspect captured patch +35 / −0
diff --git a/src/main/java/com/sparrowwallet/sparrow/io/Storage.java b/src/main/java/com/sparrowwallet/sparrow/io/Storage.java
index e5ee3f1..bae64cf 100644
--- a/src/main/java/com/sparrowwallet/sparrow/io/Storage.java
+++ b/src/main/java/com/sparrowwallet/sparrow/io/Storage.java
@@ -29,6 +29,7 @@ import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.regex.Matcher;
@@ -41,6 +42,7 @@ public class Storage {
private static final DateTimeFormatter BACKUP_DATE_FORMAT = DateTimeFormatter.ofPattern("yyyyMMddHHmmss");
private static final Pattern DATE_PATTERN = Pattern.compile(".+-([0-9]{14}?).*");
+ private static final Set<String> warnedDirectories = ConcurrentHashMap.newKeySet();
public static final String WALLETS_DIR = "wallets";
public static final String WALLETS_BACKUP_DIR = "backup";
@@ -486,12 +488,16 @@ public class Storage {
File walletsBackupDir = new File(getWalletsDir(), WALLETS_BACKUP_DIR);
if(!walletsBackupDir.exists()) {
createOwnerOnlyDirectory(walletsBackupDir);
+ } else {
+ //Unlike the wallets directory below, this directory is always created by Sparrow, so restricting it restores the permissions it was created with
+ setOwnerOnlyDirectory(walletsBackupDir);
}
return walletsBackupDir;
}
public static File getWalletsDir() {
+ boolean defaultWalletsDir = false;
File walletsDir = Config.get().getWalletsDir();
if(walletsDir != null) {
if(!walletsDir.exists() && (walletsDir.getParentFile() == null || !walletsDir.getParentFile().exists() || !walletsDir.getParentFile().canWrite())) {
@@ -501,9 +507,12 @@ public class Storage {
}
if(walletsDir == null) {
walletsDir = new File(getDataDir(), WALLETS_DIR);
+ defaultWalletsDir = true;
}
if(!walletsDir.exists()) {
createOwnerOnlyDirectory(walletsDir);
+ } else if(defaultWalletsDir) {
+ setOwnerOnlyDirectory(walletsDir);
}
return walletsDir;
@@ -700,6 +709,32 @@ public class Storage {
return false;
}
+ public static void setOwnerOnlyDirectory(File directory) {
+ //A symlinked directory has a target outside the application directories that may be deliberately shared, so leave it alone
+ if(isWindows() || Files.isSymbolicLink(directory.toPath())) {
+ return;
+ }
+
+ Set<PosixFilePermission> ownerOnly = getDirectoryOwnerOnlyPosixFilePermissions();
+ Set<PosixFilePermission> currentPermissions;
+ try {
+ currentPermissions = Files.getPosixFilePermissions(directory.toPath());
+ } catch(UnsupportedOperationException | IOException e) {
+ log.debug("Could not read permissions on directory " + directory.getAbsolutePath(), e);
+ return;
+ }
+
+ if(!ownerOnly.equals(currentPermissions)) {
+ try {
+ Files.setPosixFilePermissions(directory.toPath(), ownerOnly);
+ } catch(IOException e) {
+ if(warnedDirectories.add(directory.getAbsolutePath())) {
+ log.warn("Could not restrict permissions on directory " + directory.getAbsolutePath() + ", it remains readable by other users", e);
+ }
+ }
+ }
+ }
+
public static boolean createOwnerOnlyFile(File file) {
try {
if(isWindows()) {
Why this scored 61/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.