Skip to content

Commit ba01b3b

Browse files
BarbatosBarbatos
Barbatos
authored and
Barbatos
committed
fix(plugins): reject symlinked password/key files to prevent file disclosure
By default java.nio.file.Files.readAllBytes follows symbolic links, allowing an attacker who can plant files in a user-supplied path to redirect reads to arbitrary files (e.g. /etc/shadow, SSH private keys). The first ~1KB of the linked file would be misused as a password or private key. Introduces KeystoreCliUtils.readRegularFile that: - stats the path with LinkOption.NOFOLLOW_LINKS (lstat semantics) - rejects symlinks, directories, FIFOs and other non-regular files - opens the byte channel with LinkOption.NOFOLLOW_LINKS too, closing the TOCTOU window between stat and open - enforces a single size check via the lstat-returned attributes instead of a separate File.length() call All three call sites are migrated: - KeystoreCliUtils.readPassword (used by new/import) - KeystoreImport.readPrivateKey (key file) - KeystoreUpdate.call (password file for old+new passwords) Tests: - unit tests for readRegularFile covering success, missing file, too-large, symlink refused, directory refused, and empty file - end-to-end tests in KeystoreImportTest that provide a symlinked --key-file and --password-file and assert refusal
1 parent 6b66b40 commit ba01b3b

5 files changed

Lines changed: 218 additions & 21 deletions

File tree

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

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,18 @@
55
import java.io.File;
66
import java.io.IOException;
77
import java.io.PrintWriter;
8+
import java.nio.ByteBuffer;
9+
import java.nio.channels.SeekableByteChannel;
810
import java.nio.charset.StandardCharsets;
911
import java.nio.file.Files;
12+
import java.nio.file.LinkOption;
13+
import java.nio.file.NoSuchFileException;
14+
import java.nio.file.OpenOption;
1015
import java.nio.file.Path;
16+
import java.nio.file.StandardOpenOption;
17+
import java.nio.file.attribute.BasicFileAttributes;
1118
import java.util.Arrays;
19+
import java.util.HashSet;
1220
import java.util.LinkedHashMap;
1321
import java.util.Map;
1422
import org.tron.keystore.WalletFile;
@@ -24,18 +32,73 @@ final class KeystoreCliUtils {
2432
private KeystoreCliUtils() {
2533
}
2634

35+
/**
36+
* Read a regular file safely without following symbolic links.
37+
*
38+
* <p>This prevents an attacker who can plant files in a user-supplied
39+
* path from redirecting the read to an arbitrary file on disk (e.g. a
40+
* symlink pointing at {@code /etc/shadow} or a user's SSH private key).
41+
* Also rejects FIFOs, devices and other non-regular files.
42+
*
43+
* @param file the file to read
44+
* @param maxSize maximum acceptable file size in bytes
45+
* @param label human-readable label used in error messages
46+
* @param err writer for diagnostic messages
47+
* @return file bytes, or {@code null} if the file is missing, a symlink,
48+
* not a regular file, or too large (err is written in each case)
49+
*/
50+
static byte[] readRegularFile(File file, long maxSize, String label, PrintWriter err)
51+
throws IOException {
52+
Path path = file.toPath();
53+
54+
BasicFileAttributes attrs;
55+
try {
56+
attrs = Files.readAttributes(path, BasicFileAttributes.class,
57+
LinkOption.NOFOLLOW_LINKS);
58+
} catch (NoSuchFileException e) {
59+
err.println(label + " not found: " + file.getPath());
60+
return null;
61+
}
62+
63+
if (attrs.isSymbolicLink()) {
64+
err.println("Refusing to follow symbolic link: " + file.getPath());
65+
return null;
66+
}
67+
if (!attrs.isRegularFile()) {
68+
err.println("Not a regular file: " + file.getPath());
69+
return null;
70+
}
71+
if (attrs.size() > maxSize) {
72+
err.println(label + " too large (max " + maxSize + " bytes): " + file.getPath());
73+
return null;
74+
}
75+
76+
int size = (int) attrs.size();
77+
java.util.Set<OpenOption> openOptions = new HashSet<>();
78+
openOptions.add(StandardOpenOption.READ);
79+
openOptions.add(LinkOption.NOFOLLOW_LINKS);
80+
try (SeekableByteChannel ch = Files.newByteChannel(path, openOptions)) {
81+
ByteBuffer buf = ByteBuffer.allocate(size);
82+
while (buf.hasRemaining()) {
83+
if (ch.read(buf) < 0) {
84+
break;
85+
}
86+
}
87+
if (buf.position() < size) {
88+
byte[] actual = new byte[buf.position()];
89+
System.arraycopy(buf.array(), 0, actual, 0, buf.position());
90+
return actual;
91+
}
92+
return buf.array();
93+
}
94+
}
95+
2796
static String readPassword(File passwordFile, PrintWriter err) throws IOException {
2897
if (passwordFile != null) {
29-
if (!passwordFile.exists()) {
30-
err.println("Password file not found: " + passwordFile.getPath()
31-
+ ". Omit --password-file for interactive input.");
32-
return null;
33-
}
34-
if (passwordFile.length() > MAX_FILE_SIZE) {
35-
err.println("Password file too large (max 1KB).");
98+
byte[] bytes = readRegularFile(passwordFile, MAX_FILE_SIZE, "Password file", err);
99+
if (bytes == null) {
36100
return null;
37101
}
38-
byte[] bytes = Files.readAllBytes(passwordFile.toPath());
39102
try {
40103
String password = stripLineEndings(
41104
new String(bytes, StandardCharsets.UTF_8));

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import java.io.IOException;
66
import java.io.PrintWriter;
77
import java.nio.charset.StandardCharsets;
8-
import java.nio.file.Files;
98
import java.util.Arrays;
109
import java.util.concurrent.Callable;
1110
import org.apache.commons.lang3.StringUtils;
@@ -122,11 +121,10 @@ public Integer call() {
122121

123122
private String readPrivateKey(PrintWriter err) throws IOException {
124123
if (keyFile != null) {
125-
if (keyFile.length() > 1024) {
126-
err.println("Key file too large (max 1KB).");
124+
byte[] bytes = KeystoreCliUtils.readRegularFile(keyFile, 1024, "Key file", err);
125+
if (bytes == null) {
127126
return null;
128127
}
129-
byte[] bytes = Files.readAllBytes(keyFile.toPath());
130128
try {
131129
return new String(bytes, StandardCharsets.UTF_8).trim();
132130
} finally {

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import java.io.IOException;
77
import java.io.PrintWriter;
88
import java.nio.charset.StandardCharsets;
9-
import java.nio.file.Files;
109
import java.util.Arrays;
1110
import java.util.concurrent.Callable;
1211
import org.tron.common.crypto.SignInterface;
@@ -66,16 +65,11 @@ public Integer call() {
6665
String newPassword;
6766

6867
if (passwordFile != null) {
69-
if (!passwordFile.exists()) {
70-
err.println("Password file not found: " + passwordFile.getPath()
71-
+ ". Omit --password-file for interactive input.");
68+
byte[] bytes = KeystoreCliUtils.readRegularFile(
69+
passwordFile, 1024, "Password file", err);
70+
if (bytes == null) {
7271
return 1;
7372
}
74-
if (passwordFile.length() > 1024) {
75-
err.println("Password file too large (max 1KB).");
76-
return 1;
77-
}
78-
byte[] bytes = Files.readAllBytes(passwordFile.toPath());
7973
try {
8074
String content = new String(bytes, StandardCharsets.UTF_8);
8175
// Strip UTF-8 BOM if present (Windows Notepad)

‎plugins/src/test/java/org/tron/plugins/KeystoreCliUtilsTest.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,4 +256,84 @@ public void testPrintSecurityTipsIncludesAddressAndFile() {
256256
assertTrue(s.contains("REMEMBER"));
257257
}
258258

259+
@Test
260+
public void testReadRegularFileSuccess() throws Exception {
261+
File f = tempFolder.newFile("regular.txt");
262+
Files.write(f.toPath(), "hello".getBytes(StandardCharsets.UTF_8));
263+
StringWriter err = new StringWriter();
264+
265+
byte[] bytes = KeystoreCliUtils.readRegularFile(f, 1024, "File",
266+
new PrintWriter(err));
267+
assertNotNull(bytes);
268+
assertEquals("hello", new String(bytes, StandardCharsets.UTF_8));
269+
}
270+
271+
@Test
272+
public void testReadRegularFileMissing() throws Exception {
273+
File f = new File(tempFolder.getRoot(), "does-not-exist");
274+
StringWriter err = new StringWriter();
275+
276+
byte[] bytes = KeystoreCliUtils.readRegularFile(f, 1024, "Password file",
277+
new PrintWriter(err));
278+
assertNull(bytes);
279+
assertTrue("Expected 'not found' error, got: " + err.toString(),
280+
err.toString().contains("Password file not found"));
281+
}
282+
283+
@Test
284+
public void testReadRegularFileTooLarge() throws Exception {
285+
File f = tempFolder.newFile("big.txt");
286+
byte[] big = new byte[2048];
287+
java.util.Arrays.fill(big, (byte) 'a');
288+
Files.write(f.toPath(), big);
289+
StringWriter err = new StringWriter();
290+
291+
byte[] bytes = KeystoreCliUtils.readRegularFile(f, 1024, "Password file",
292+
new PrintWriter(err));
293+
assertNull(bytes);
294+
assertTrue("Expected 'too large', got: " + err.toString(),
295+
err.toString().contains("too large"));
296+
}
297+
298+
@Test
299+
public void testReadRegularFileRefusesSymlink() throws Exception {
300+
org.junit.Assume.assumeTrue("Symlinks only tested on POSIX",
301+
!System.getProperty("os.name").toLowerCase().contains("win"));
302+
303+
File target = tempFolder.newFile("real-target.txt");
304+
Files.write(target.toPath(), "secret content".getBytes(StandardCharsets.UTF_8));
305+
File link = new File(tempFolder.getRoot(), "symlink.txt");
306+
Files.createSymbolicLink(link.toPath(), target.toPath());
307+
308+
StringWriter err = new StringWriter();
309+
byte[] bytes = KeystoreCliUtils.readRegularFile(link, 1024, "File",
310+
new PrintWriter(err));
311+
312+
assertNull("Must refuse to read through symlink", bytes);
313+
assertTrue("Expected symlink-refusal message, got: " + err.toString(),
314+
err.toString().contains("Refusing to follow symbolic link"));
315+
}
316+
317+
@Test
318+
public void testReadRegularFileRefusesDirectory() throws Exception {
319+
File dir = tempFolder.newFolder("a-dir");
320+
StringWriter err = new StringWriter();
321+
322+
byte[] bytes = KeystoreCliUtils.readRegularFile(dir, 1024, "File",
323+
new PrintWriter(err));
324+
assertNull(bytes);
325+
assertTrue("Expected not-regular-file error, got: " + err.toString(),
326+
err.toString().contains("Not a regular file"));
327+
}
328+
329+
@Test
330+
public void testReadRegularFileEmptyFile() throws Exception {
331+
File f = tempFolder.newFile("empty.txt");
332+
StringWriter err = new StringWriter();
333+
334+
byte[] bytes = KeystoreCliUtils.readRegularFile(f, 1024, "File",
335+
new PrintWriter(err));
336+
assertNotNull(bytes);
337+
assertEquals(0, bytes.length);
338+
}
259339
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,68 @@ public void testImportKeystoreFilePermissions() throws Exception {
374374
perms);
375375
}
376376

377+
@Test
378+
public void testImportRefusesSymlinkKeyFile() throws Exception {
379+
org.junit.Assume.assumeTrue("Symlinks only tested on POSIX",
380+
!System.getProperty("os.name").toLowerCase().contains("win"));
381+
382+
File dir = tempFolder.newFolder("keystore-symlink");
383+
// Create a real key file and a symlink pointing to it
384+
File target = tempFolder.newFile("real.key");
385+
SignInterface keyPair = SignUtils.getGeneratedRandomSign(
386+
SecureRandom.getInstance("NativePRNG"), true);
387+
Files.write(target.toPath(),
388+
ByteArray.toHexString(keyPair.getPrivateKey()).getBytes(StandardCharsets.UTF_8));
389+
390+
File symlink = new File(tempFolder.getRoot(), "symlink.key");
391+
Files.createSymbolicLink(symlink.toPath(), target.toPath());
392+
393+
File pwFile = tempFolder.newFile("pw-symlink.txt");
394+
Files.write(pwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8));
395+
396+
java.io.StringWriter err = new java.io.StringWriter();
397+
CommandLine cmd = new CommandLine(new Toolkit());
398+
cmd.setErr(new java.io.PrintWriter(err));
399+
int exitCode = cmd.execute("keystore", "import",
400+
"--keystore-dir", dir.getAbsolutePath(),
401+
"--key-file", symlink.getAbsolutePath(),
402+
"--password-file", pwFile.getAbsolutePath());
403+
404+
assertEquals("Must refuse symlinked key file", 1, exitCode);
405+
assertTrue("Expected symlink-refusal error, got: " + err.toString(),
406+
err.toString().contains("Refusing to follow symbolic link"));
407+
}
408+
409+
@Test
410+
public void testImportRefusesSymlinkPasswordFile() throws Exception {
411+
org.junit.Assume.assumeTrue("Symlinks only tested on POSIX",
412+
!System.getProperty("os.name").toLowerCase().contains("win"));
413+
414+
File dir = tempFolder.newFolder("keystore-pwsymlink");
415+
SignInterface keyPair = SignUtils.getGeneratedRandomSign(
416+
SecureRandom.getInstance("NativePRNG"), true);
417+
File keyFile = tempFolder.newFile("sym-pw.key");
418+
Files.write(keyFile.toPath(),
419+
ByteArray.toHexString(keyPair.getPrivateKey()).getBytes(StandardCharsets.UTF_8));
420+
421+
File realPwFile = tempFolder.newFile("real-pw.txt");
422+
Files.write(realPwFile.toPath(), "test123456".getBytes(StandardCharsets.UTF_8));
423+
File pwSymlink = new File(tempFolder.getRoot(), "pw-symlink.txt");
424+
Files.createSymbolicLink(pwSymlink.toPath(), realPwFile.toPath());
425+
426+
java.io.StringWriter err = new java.io.StringWriter();
427+
CommandLine cmd = new CommandLine(new Toolkit());
428+
cmd.setErr(new java.io.PrintWriter(err));
429+
int exitCode = cmd.execute("keystore", "import",
430+
"--keystore-dir", dir.getAbsolutePath(),
431+
"--key-file", keyFile.getAbsolutePath(),
432+
"--password-file", pwSymlink.getAbsolutePath());
433+
434+
assertEquals("Must refuse symlinked password file", 1, exitCode);
435+
assertTrue("Expected symlink-refusal error, got: " + err.toString(),
436+
err.toString().contains("Refusing to follow symbolic link"));
437+
}
438+
377439
@Test
378440
public void testImportDuplicateCheckSkipsInvalidVersion() throws Exception {
379441
File dir = tempFolder.newFolder("keystore-badver");

0 commit comments

Comments
 (0)