From 19eb5a1876261cac982812d130013b8827c78dd7 Mon Sep 17 00:00:00 2001
From: Jun Luo <4catcode@gmail.com>
Date: Thu, 28 Sep 2023 06:55:49 +0800
Subject: [PATCH] Fix the issue where the validity of the encoded strkey is not
 verified in certain scenarios. (#541)

---
 CHANGELOG.md                                  |  1 +
 src/main/java/org/stellar/sdk/StrKey.java     | 15 +++-
 src/test/java/org/stellar/sdk/StrKeyTest.java | 89 ++++++++++++++++++-
 3 files changed, 102 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9d75951c1..00b3b5608 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 As this project is pre 1.0, breaking changes may happen for minor version bumps. A breaking change will get clearly notified in this log.
 
 ## Pending
+* Fix issues where the validity of the encoded strkey is not verified in certain scenarios. ([#541](https://github.com/stellar/java-stellar-sdk/pull/541))
 * Fix the issue of javadocJar not including documentation. ([#539](https://github.com/stellar/java-stellar-sdk/pull/539))
 * Publish sourcesJar to the GitHub Release page. ([#539](https://github.com/stellar/java-stellar-sdk/pull/539))
 
diff --git a/src/main/java/org/stellar/sdk/StrKey.java b/src/main/java/org/stellar/sdk/StrKey.java
index 7856fef90..dceabd206 100644
--- a/src/main/java/org/stellar/sdk/StrKey.java
+++ b/src/main/java/org/stellar/sdk/StrKey.java
@@ -7,6 +7,7 @@
 import java.util.Optional;
 import org.apache.commons.codec.binary.Base32;
 import org.apache.commons.codec.binary.Base32OutputStream;
+import org.apache.commons.codec.binary.StringUtils;
 import org.stellar.sdk.xdr.AccountID;
 import org.stellar.sdk.xdr.CryptoKeyType;
 import org.stellar.sdk.xdr.MuxedAccount;
@@ -139,7 +140,8 @@ public static MuxedAccount encodeToXDRMuxedAccount(String data) {
   }
 
   public static VersionByte decodeVersionByte(String data) {
-    byte[] decoded = base32Codec.decode(data);
+    byte[] dataBytes = StringUtils.getBytesUtf8(data);
+    byte[] decoded = base32decode(dataBytes);
     byte decodedVersionByte = decoded[0];
     Optional<VersionByte> versionByteOptional = VersionByte.findByValue(decodedVersionByte);
     if (!versionByteOptional.isPresent()) {
@@ -300,7 +302,7 @@ protected static byte[] decodeCheck(VersionByte versionByte, char[] encoded) {
       }
     }
 
-    byte[] decoded = base32Codec.decode(bytes);
+    byte[] decoded = base32decode(bytes);
     byte decodedVersionByte = decoded[0];
     byte[] payload = Arrays.copyOfRange(decoded, 0, decoded.length - 2);
     byte[] data = Arrays.copyOfRange(payload, 1, payload.length);
@@ -424,4 +426,13 @@ private static char[] bytesToChars(byte[] data) {
     }
     return chars;
   }
+
+  private static byte[] base32decode(byte[] data) {
+    // Apache commons codec Base32 class will auto remove the illegal characters, this is
+    // what we don't want, so we need to check the data before decoding
+    if (!base32Codec.isInAlphabet(data, false)) {
+      throw new IllegalArgumentException("Invalid base32 encoded string");
+    }
+    return base32Codec.decode(data);
+  }
 }
diff --git a/src/test/java/org/stellar/sdk/StrKeyTest.java b/src/test/java/org/stellar/sdk/StrKeyTest.java
index 7a8cff31c..b11eebb60 100644
--- a/src/test/java/org/stellar/sdk/StrKeyTest.java
+++ b/src/test/java/org/stellar/sdk/StrKeyTest.java
@@ -333,7 +333,7 @@ public void testEncodeAccountIdToMuxed() {
 
   @Test
   public void testEncodeToXDRMuxedAccountInvalidAddress() {
-
+    // https://github.com/stellar/go/blob/2b876cd781b6dd0c218dcdd4f300900f87b3889e/strkey/main_test.go#L86
     try {
       StrKey.encodeToXDRMuxedAccount("XBU2RRGLXH3E5CQHTD3ODLDF2BWDCYUSSBLLZ5GNW7JXHDIYKXZWGTOG");
       fail();
@@ -347,5 +347,92 @@ public void testEncodeToXDRMuxedAccountInvalidAddress() {
     } catch (FormatException e) {
       assertEquals("Checksum invalid", e.getMessage());
     }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount(
+          "MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJUAAAAAAAAAAAACJUR");
+      fail();
+    } catch (IllegalArgumentException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount("GA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVSGZA");
+      fail();
+    } catch (IllegalArgumentException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount("G47QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVP2I");
+      fail();
+    } catch (FormatException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount(
+          "MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVAAAAAAAAAAAAAJLKA");
+      fail();
+    } catch (IllegalArgumentException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount(
+          "M47QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJUAAAAAAAAAAAACJUQ");
+      fail();
+    } catch (FormatException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount(
+          "M47QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJUAAAAAAAAAAAACJUQ");
+      fail();
+    } catch (FormatException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount(
+          "MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJUAAAAAAAAAAAACJUK");
+      fail();
+    } catch (FormatException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount(
+          "MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJUAAAAAAAAAAAACJUO");
+      fail();
+    } catch (FormatException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount("");
+      fail();
+    } catch (IllegalArgumentException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount("SBCVMMCBEDB64TVJZFYJOJAERZC4YVVUOE6SYR2Y76CBTENGUSGWRRVO");
+      fail();
+    } catch (FormatException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount(
+          "MCEO75Y6YKE53HM6N46IJYH3LK3YYFZ4QWGNUKCSSIQSH3KOAD7BEAAAAAAAAAAAPNT2W___");
+      fail();
+    } catch (IllegalArgumentException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount(
+          "MDWZCOEQRODFCH6ISYQPWY67L3ULLWS5ISXYYL5GH43W7YFMTLB64AAAAAAAAAAAAHGLW===");
+      fail();
+    } catch (IllegalArgumentException ignored) {
+    }
+
+    try {
+      StrKey.encodeToXDRMuxedAccount(
+          " MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVAAAAAAAAAAAAAJLK");
+      fail();
+    } catch (IllegalArgumentException ignored) {
+    }
   }
 }