Skip to content

Commit 340669c

Browse files
BarbatosBarbatos
authored andcommitted
fix(plugins): reject symlinks in keystore directory scans
Earlier commits hardened --password-file / --key-file reads against symlink-following, but the keystore-directory scans in list, import (duplicate check), and update (findKeystoreByAddress) still called MAPPER.readValue(file, ...) on every *.json entry without a symlink guard. In a hostile or group-writable keystore directory, a planted symlink could redirect deserialization to arbitrary files, and FIFOs or devices could block the scan indefinitely. - Add KeystoreCliUtils.isSafeRegularFile(file, err) helper that stats with LinkOption.NOFOLLOW_LINKS and rejects symbolic links, directories, FIFOs, and devices with a warning to err - Apply it before MAPPER.readValue in KeystoreList.call, KeystoreImport.findExistingKeystore, and KeystoreUpdate.findKeystoreByAddress - Add three end-to-end tests (one per command) that plant a .json symlink in the keystore dir and verify each command warns and continues without reading the symlink target
1 parent 99159e2 commit 340669c

7 files changed

Lines changed: 151 additions & 0 deletions

File tree

plugins/src/main/java/common/org/tron/plugins/KeystoreCliUtils.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,35 @@ static boolean checkFileExists(File file, String label, PrintWriter err) {
188188
return true;
189189
}
190190

191+
/**
192+
* Returns true iff {@code file} exists, is not a symbolic link, and is a
193+
* regular file (not a directory, FIFO, or device). Used to filter keystore
194+
* directory scans before {@code MAPPER.readValue(file, ...)} so a hostile
195+
* or group-writable keystore directory cannot redirect reads to arbitrary
196+
* files (e.g. {@code /etc/shadow}) or block on non-regular files
197+
* (e.g. a FIFO) via planted entries.
198+
*
199+
* <p>Writes a warning to {@code err} when the entry is skipped.
200+
*/
201+
static boolean isSafeRegularFile(File file, PrintWriter err) {
202+
try {
203+
BasicFileAttributes attrs = Files.readAttributes(file.toPath(),
204+
BasicFileAttributes.class, LinkOption.NOFOLLOW_LINKS);
205+
if (attrs.isSymbolicLink()) {
206+
err.println("Warning: skipping symbolic link: " + file.getName());
207+
return false;
208+
}
209+
if (!attrs.isRegularFile()) {
210+
err.println("Warning: skipping non-regular file: " + file.getName());
211+
return false;
212+
}
213+
return true;
214+
} catch (IOException e) {
215+
err.println("Warning: skipping unreadable file: " + file.getName());
216+
return false;
217+
}
218+
}
219+
191220
static void printSecurityTips(PrintWriter out, String address, String fileName) {
192221
out.println();
193222
out.println("Public address of the key: " + address);

plugins/src/main/java/common/org/tron/plugins/KeystoreImport.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,9 @@ private String findExistingKeystore(File dir, String address, PrintWriter err) {
169169
com.fasterxml.jackson.databind.ObjectMapper mapper =
170170
KeystoreCliUtils.mapper();
171171
for (File file : files) {
172+
if (!KeystoreCliUtils.isSafeRegularFile(file, err)) {
173+
continue;
174+
}
172175
try {
173176
WalletFile wf = mapper.readValue(file, WalletFile.class);
174177
if (KeystoreCliUtils.isValidKeystoreFile(wf)

plugins/src/main/java/common/org/tron/plugins/KeystoreList.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ public Integer call() {
5959

6060
List<Map<String, String>> entries = new ArrayList<>();
6161
for (File file : files) {
62+
if (!KeystoreCliUtils.isSafeRegularFile(file, err)) {
63+
continue;
64+
}
6265
try {
6366
WalletFile walletFile = MAPPER.readValue(file, WalletFile.class);
6467
if (!KeystoreCliUtils.isValidKeystoreFile(walletFile)) {

plugins/src/main/java/common/org/tron/plugins/KeystoreUpdate.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ private File findKeystoreByAddress(String targetAddress, PrintWriter err) {
199199
}
200200
java.util.List<File> matches = new java.util.ArrayList<>();
201201
for (File file : files) {
202+
if (!KeystoreCliUtils.isSafeRegularFile(file, err)) {
203+
continue;
204+
}
202205
try {
203206
WalletFile wf = MAPPER.readValue(file, WalletFile.class);
204207
if (KeystoreCliUtils.isValidKeystoreFile(wf)

plugins/src/test/java/org/tron/plugins/KeystoreImportTest.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,4 +464,41 @@ public void testImportDuplicateCheckSkipsInvalidVersion() throws Exception {
464464
assertEquals("Import should succeed — invalid-version file is not a real duplicate", 0,
465465
exitCode);
466466
}
467+
468+
@Test
469+
public void testImportDuplicateScanSkipsSymlinkedEntry() throws Exception {
470+
org.junit.Assume.assumeTrue("Symlinks only tested on POSIX",
471+
!System.getProperty("os.name").toLowerCase().contains("win"));
472+
473+
File dir = tempFolder.newFolder("keystore-dup-symlink");
474+
475+
SignInterface keyPair = SignUtils.getGeneratedRandomSign(
476+
SecureRandom.getInstance("NativePRNG"), true);
477+
String privateKeyHex = ByteArray.toHexString(keyPair.getPrivateKey());
478+
479+
File target = tempFolder.newFile("outside.json");
480+
Files.write(target.toPath(),
481+
"{\"not\":\"a keystore\"}".getBytes(StandardCharsets.UTF_8));
482+
File symlink = new File(dir, "evil.json");
483+
Files.createSymbolicLink(symlink.toPath(), target.toPath());
484+
485+
File keyFile = tempFolder.newFile("dup-sym.key");
486+
Files.write(keyFile.toPath(),
487+
privateKeyHex.getBytes(StandardCharsets.UTF_8));
488+
File pwFile = tempFolder.newFile("pw-dup-sym.txt");
489+
Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8));
490+
491+
java.io.StringWriter err = new java.io.StringWriter();
492+
CommandLine cmd = new CommandLine(new Toolkit());
493+
cmd.setErr(new java.io.PrintWriter(err));
494+
int exitCode = cmd.execute("keystore", "import",
495+
"--keystore-dir", dir.getAbsolutePath(),
496+
"--key-file", keyFile.getAbsolutePath(),
497+
"--password-file", pwFile.getAbsolutePath());
498+
499+
assertEquals("Import should succeed with symlink present", 0, exitCode);
500+
assertTrue("Duplicate scan must warn about the symlinked entry, got: "
501+
+ err.toString(),
502+
err.toString().contains("Warning: skipping symbolic link: evil.json"));
503+
}
467504
}

plugins/src/test/java/org/tron/plugins/KeystoreListTest.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,4 +240,43 @@ public void testListSkipsInvalidVersionKeystores() throws Exception {
240240
assertEquals("Should list only the valid v3 keystore, not v2 or no-crypto",
241241
1, lines.length);
242242
}
243+
244+
@Test
245+
public void testListSkipsSymlinkedKeystoreFile() throws Exception {
246+
org.junit.Assume.assumeTrue("Symlinks only tested on POSIX",
247+
!System.getProperty("os.name").toLowerCase().contains("win"));
248+
249+
File dir = tempFolder.newFolder("keystore-symlink-scan");
250+
String password = "test123456";
251+
252+
SignInterface key = SignUtils.getGeneratedRandomSign(
253+
SecureRandom.getInstance("NativePRNG"), true);
254+
WalletUtils.generateWalletFile(password, key, dir, false);
255+
256+
// A JSON file elsewhere (simulates "target we should not be tricked
257+
// into reading") — placed outside the keystore dir.
258+
File target = tempFolder.newFile("outside.json");
259+
Files.write(target.toPath(),
260+
"{\"secret\":\"should not appear in list output\"}"
261+
.getBytes(StandardCharsets.UTF_8));
262+
263+
// Plant a .json symlink in the keystore dir
264+
File symlink = new File(dir, "evil.json");
265+
Files.createSymbolicLink(symlink.toPath(), target.toPath());
266+
267+
StringWriter out = new StringWriter();
268+
StringWriter err = new StringWriter();
269+
CommandLine cmd = new CommandLine(new Toolkit());
270+
cmd.setOut(new PrintWriter(out));
271+
cmd.setErr(new PrintWriter(err));
272+
int exitCode = cmd.execute("keystore", "list",
273+
"--keystore-dir", dir.getAbsolutePath());
274+
275+
assertEquals(0, exitCode);
276+
assertTrue("Should warn about symbolic link, got err: " + err.toString(),
277+
err.toString().contains("Warning: skipping symbolic link: evil.json"));
278+
String output = out.toString().trim();
279+
String[] lines = output.split("\\n");
280+
assertEquals("Should list only the real keystore", 1, lines.length);
281+
}
243282
}

plugins/src/test/java/org/tron/plugins/KeystoreUpdateTest.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,4 +736,41 @@ public void testUpdateLegacyTipSuppressedWhenPasswordHasNoWhitespace() throws Ex
736736
assertFalse("Legacy-truncation tip should NOT fire for whitespace-free password",
737737
err.toString().contains("first whitespace-separated word"));
738738
}
739+
740+
@Test
741+
public void testUpdateScanSkipsSymlinkedEntry() throws Exception {
742+
org.junit.Assume.assumeTrue("Symlinks only tested on POSIX",
743+
!System.getProperty("os.name").toLowerCase().contains("win"));
744+
745+
File dir = tempFolder.newFolder("keystore-update-symlink");
746+
String oldPassword = "oldpass123";
747+
String newPassword = "newpass456";
748+
749+
SignInterface keyPair = SignUtils.getGeneratedRandomSign(
750+
SecureRandom.getInstance("NativePRNG"), true);
751+
String fileName = WalletUtils.generateWalletFile(oldPassword, keyPair, dir, true);
752+
Credentials creds = WalletUtils.loadCredentials(oldPassword,
753+
new File(dir, fileName), true);
754+
755+
File target = tempFolder.newFile("outside.json");
756+
Files.write(target.toPath(),
757+
"{\"not\":\"a keystore\"}".getBytes(StandardCharsets.UTF_8));
758+
File symlink = new File(dir, "evil.json");
759+
Files.createSymbolicLink(symlink.toPath(), target.toPath());
760+
761+
File pwFile = tempFolder.newFile("pw-update-sym.txt");
762+
Files.write(pwFile.toPath(),
763+
(oldPassword + "\n" + newPassword).getBytes(StandardCharsets.UTF_8));
764+
765+
StringWriter err = new StringWriter();
766+
CommandLine cmd = new CommandLine(new Toolkit());
767+
cmd.setErr(new PrintWriter(err));
768+
int exitCode = cmd.execute("keystore", "update", creds.getAddress(),
769+
"--keystore-dir", dir.getAbsolutePath(),
770+
"--password-file", pwFile.getAbsolutePath());
771+
772+
assertEquals("Update should succeed; symlinked entry must not break scan", 0, exitCode);
773+
assertTrue("Scan must warn about the symlinked entry, got: " + err.toString(),
774+
err.toString().contains("Warning: skipping symbolic link: evil.json"));
775+
}
739776
}

0 commit comments

Comments
 (0)