Skip to content

Commit 51350e8

Browse files
BarbatosBarbatos
authored andcommitted
fix(plugins): prevent address spoofing in keystore update command
- Remove newWalletFile.setAddress(walletFile.getAddress()) — createStandard already sets the correctly-derived address, and propagating the JSON address could carry a spoofed value forward - Use the derived address from newWalletFile for output messages instead of walletFile.getAddress(), keeping the output correct as a defense-in-depth measure even if upstream validation is weakened - Add tests for tampered-address rejection and derived-address output
1 parent 63bf397 commit 51350e8

2 files changed

Lines changed: 84 additions & 3 deletions

File tree

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,10 @@ public Integer call() {
144144
WalletFile walletFile = MAPPER.readValue(keystoreFile, WalletFile.class);
145145
SignInterface keyPair = Wallet.decrypt(oldPassword, walletFile, ecKey);
146146

147+
// createStandard already sets the correctly-derived address. Do NOT override
148+
// with walletFile.getAddress() — that would propagate a potentially spoofed
149+
// address from the JSON.
147150
WalletFile newWalletFile = Wallet.createStandard(newPassword, keyPair);
148-
newWalletFile.setAddress(walletFile.getAddress());
149151
// Write to temp file first, then atomic rename to prevent corruption
150152
File tempFile = File.createTempFile("keystore-", ".tmp",
151153
keystoreFile.getParentFile());
@@ -161,13 +163,18 @@ public Integer call() {
161163
throw e;
162164
}
163165

166+
// Use the derived address from newWalletFile, not walletFile.getAddress().
167+
// Defense-in-depth: Wallet.decrypt already rejects spoofed addresses, but
168+
// relying on the derived value keeps this code correct even if that check
169+
// is ever weakened.
170+
String verifiedAddress = newWalletFile.getAddress();
164171
if (json) {
165172
KeystoreCliUtils.printJson(out, err, KeystoreCliUtils.jsonMap(
166-
"address", walletFile.getAddress(),
173+
"address", verifiedAddress,
167174
"file", keystoreFile.getName(),
168175
"status", "updated"));
169176
} else {
170-
out.println("Password updated for: " + walletFile.getAddress());
177+
out.println("Password updated for: " + verifiedAddress);
171178
}
172179
return 0;
173180
} catch (CipherException e) {

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,4 +548,78 @@ public void testUpdateSkipsInvalidVersionKeystores() throws Exception {
548548
assertTrue("Error should mention no keystore found",
549549
err.toString().contains("No keystore found"));
550550
}
551+
552+
@Test
553+
public void testUpdateRejectsTamperedAddressKeystore() throws Exception {
554+
File dir = tempFolder.newFolder("keystore-tampered");
555+
String password = "test123456";
556+
557+
// Create a real keystore, then tamper with the address field to simulate
558+
// a spoofed keystore that claims a different address than its encrypted key.
559+
SignInterface keyPair = SignUtils.getGeneratedRandomSign(
560+
SecureRandom.getInstance("NativePRNG"), true);
561+
String fileName = WalletUtils.generateWalletFile(password, keyPair, dir, true);
562+
File keystoreFile = new File(dir, fileName);
563+
564+
String realAddress = Credentials.create(keyPair).getAddress();
565+
String spoofedAddress = "TSpoofedAddressXXXXXXXXXXXXXXXXXXXX";
566+
567+
com.fasterxml.jackson.databind.ObjectMapper mapper =
568+
new com.fasterxml.jackson.databind.ObjectMapper()
569+
.configure(com.fasterxml.jackson.databind.DeserializationFeature
570+
.FAIL_ON_UNKNOWN_PROPERTIES, false);
571+
org.tron.keystore.WalletFile wf = mapper.readValue(keystoreFile,
572+
org.tron.keystore.WalletFile.class);
573+
wf.setAddress(spoofedAddress);
574+
mapper.writeValue(keystoreFile, wf);
575+
576+
File pwFile = tempFolder.newFile("pw-tampered.txt");
577+
Files.write(pwFile.toPath(),
578+
(password + "\nnewpass789").getBytes(StandardCharsets.UTF_8));
579+
580+
StringWriter err = new StringWriter();
581+
CommandLine cmd = new CommandLine(new Toolkit());
582+
cmd.setErr(new PrintWriter(err));
583+
int exitCode = cmd.execute("keystore", "update", spoofedAddress,
584+
"--keystore-dir", dir.getAbsolutePath(),
585+
"--password-file", pwFile.getAbsolutePath());
586+
587+
assertEquals("Should fail decryption on tampered address", 1, exitCode);
588+
assertTrue("Error should mention address mismatch, got: " + err.toString(),
589+
err.toString().contains("address mismatch"));
590+
}
591+
592+
@Test
593+
public void testUpdatePreservesCorrectDerivedAddress() throws Exception {
594+
// After update, the keystore's address field should be the derived address,
595+
// not carried over from the original JSON (defense-in-depth against any
596+
// residual spoofed address that somehow passed decryption).
597+
File dir = tempFolder.newFolder("keystore-derived");
598+
String oldPassword = "oldpass123";
599+
String newPassword = "newpass456";
600+
601+
SignInterface keyPair = SignUtils.getGeneratedRandomSign(
602+
SecureRandom.getInstance("NativePRNG"), true);
603+
String fileName = WalletUtils.generateWalletFile(oldPassword, keyPair, dir, true);
604+
String originalAddress = Credentials.create(keyPair).getAddress();
605+
606+
File pwFile = tempFolder.newFile("pw-derived.txt");
607+
Files.write(pwFile.toPath(),
608+
(oldPassword + "\n" + newPassword).getBytes(StandardCharsets.UTF_8));
609+
610+
CommandLine cmd = new CommandLine(new Toolkit());
611+
int exitCode = cmd.execute("keystore", "update", originalAddress,
612+
"--keystore-dir", dir.getAbsolutePath(),
613+
"--password-file", pwFile.getAbsolutePath());
614+
615+
assertEquals(0, exitCode);
616+
617+
// Verify updated file has the derived address
618+
com.fasterxml.jackson.databind.ObjectMapper mapper =
619+
new com.fasterxml.jackson.databind.ObjectMapper();
620+
org.tron.keystore.WalletFile wf = mapper.readValue(new File(dir, fileName),
621+
org.tron.keystore.WalletFile.class);
622+
assertEquals("Updated keystore address must match derived address",
623+
originalAddress, wf.getAddress());
624+
}
551625
}

0 commit comments

Comments
 (0)