From 95112af2a1b79ca808695268b537e0b0bc0a6322 Mon Sep 17 00:00:00 2001 From: Jun Luo <4catcode@gmail.com> Date: Fri, 30 Jan 2026 22:56:07 +0800 Subject: [PATCH] fix: add stricter validation for Ed25519 Signed Payload --- CHANGELOG.md | 3 + src/main/java/org/stellar/sdk/SignerKey.java | 45 ++++- src/main/java/org/stellar/sdk/StrKey.java | 16 ++ .../java/org/stellar/sdk/SignerKeyTest.java | 173 +++++++++++++++++- src/test/kotlin/org/stellar/sdk/StrKeyTest.kt | 61 +++++- 5 files changed, 292 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b9fcfea6..0fa9777d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Pending +### Update +- fix: add stricter validation for Ed25519 Signed Payload. + ## 2.2.1 ### Update: diff --git a/src/main/java/org/stellar/sdk/SignerKey.java b/src/main/java/org/stellar/sdk/SignerKey.java index b5ac148d8..c55973a61 100644 --- a/src/main/java/org/stellar/sdk/SignerKey.java +++ b/src/main/java/org/stellar/sdk/SignerKey.java @@ -279,9 +279,43 @@ public Ed25519SignedPayload toEd25519SignedPayload() { "SignerKey type must be SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD"); } + // Validate key length: min 40 bytes (32 + 4 + 4), max 100 bytes (32 + 4 + 64) + if (key.length < 40 || key.length > 100) { + throw new IllegalArgumentException( + "Invalid signed payload key length, must be between 40 and 100 bytes, got " + key.length); + } + byte[] lengthBytes = Arrays.copyOfRange(key, 32, 36); int payloadLength = ByteBuffer.wrap(lengthBytes).getInt(); + // Validate payload length: must be between 1 and 64 bytes + if (payloadLength < 1 || payloadLength > SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH) { + throw new IllegalArgumentException( + "Invalid payload length, must be between 1 and " + + SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH + + ", got " + + payloadLength); + } + + // Validate total length matches expected (32 + 4 + payloadLength + padding) + int padding = (4 - payloadLength % 4) % 4; + int expectedLength = 32 + 4 + payloadLength + padding; + if (key.length != expectedLength) { + throw new IllegalArgumentException( + "Invalid signed payload key length, expected " + + expectedLength + + " bytes, got " + + key.length); + } + + // Validate padding bytes are all zeros + for (int i = 36 + payloadLength; i < key.length; i++) { + if (key[i] != 0) { + throw new IllegalArgumentException( + "Invalid signed payload key, padding bytes must be zero"); + } + } + byte[] publicKeyBytes = Arrays.copyOfRange(key, 0, 32); byte[] payload = Arrays.copyOfRange(key, 36, 36 + payloadLength); return new Ed25519SignedPayload(publicKeyBytes, payload); @@ -372,13 +406,16 @@ public static class Ed25519SignedPayload { * Creates a new Ed25519SignedPayload. * * @param ed25519PublicKey The 32-byte Ed25519 public key - * @param payload The payload to be signed (maximum 64 bytes) - * @throws IllegalArgumentException if the payload length exceeds the maximum allowed + * @param payload The payload to be signed (1-64 bytes) + * @throws IllegalArgumentException if the payload length is not between 1 and 64 */ public Ed25519SignedPayload(byte[] ed25519PublicKey, byte[] payload) { - if (payload.length > SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH) { + if (payload.length < 1 || payload.length > SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH) { throw new IllegalArgumentException( - "Invalid payload length, must be less than " + SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH); + "Invalid payload length, must be between 1 and " + + SIGNED_PAYLOAD_MAX_PAYLOAD_LENGTH + + ", got " + + payload.length); } this.ed25519PublicKey = ed25519PublicKey; this.payload = payload; diff --git a/src/main/java/org/stellar/sdk/StrKey.java b/src/main/java/org/stellar/sdk/StrKey.java index b0e165c58..191e0f94c 100644 --- a/src/main/java/org/stellar/sdk/StrKey.java +++ b/src/main/java/org/stellar/sdk/StrKey.java @@ -609,10 +609,26 @@ static byte[] decodeCheck(VersionByte versionByte, char[] encoded) { if (VersionByte.SIGNED_PAYLOAD.getValue() == decodedVersionByte) { byte[] lengthBytes = Arrays.copyOfRange(data, 32, 36); int payloadLength = ByteBuffer.wrap(lengthBytes).getInt(); + + // Validate payload length: must be between 1 and 64 bytes + if (payloadLength < 1 || payloadLength > 64) { + throw new IllegalArgumentException( + "Invalid Ed25519 Signed Payload Key, payload length must be between 1 and 64, got " + + payloadLength); + } + int padding = (4 - payloadLength % 4) % 4; if (data.length % 4 != 0 || payloadLength + padding != data.length - 36) { throw new IllegalArgumentException("Invalid Ed25519 Signed Payload Key"); } + + // Validate padding bytes are all zeros + for (int i = 36 + payloadLength; i < data.length; i++) { + if (data[i] != 0) { + throw new IllegalArgumentException( + "Invalid Ed25519 Signed Payload Key, padding bytes must be zero"); + } + } } if (VersionByte.SEED.getValue() == decodedVersionByte) { diff --git a/src/test/java/org/stellar/sdk/SignerKeyTest.java b/src/test/java/org/stellar/sdk/SignerKeyTest.java index 01a17fa31..5c5f9fcb6 100644 --- a/src/test/java/org/stellar/sdk/SignerKeyTest.java +++ b/src/test/java/org/stellar/sdk/SignerKeyTest.java @@ -197,6 +197,7 @@ public void testToEd25519SignedPayloadWithWrongType() { public void testEd25519SignedPayloadConstructorValidation() { byte[] publicKey = new byte[32]; byte[] validPayload = new byte[64]; + byte[] emptyPayload = new byte[0]; // Too small byte[] invalidPayload = new byte[65]; // Too large // Valid construction @@ -205,12 +206,20 @@ public void testEd25519SignedPayloadConstructorValidation() { assertArrayEquals(publicKey, validSignedPayload.getEd25519PublicKey()); assertArrayEquals(validPayload, validSignedPayload.getPayload()); + // Invalid construction - payload too small (empty) + try { + new SignerKey.Ed25519SignedPayload(publicKey, emptyPayload); + fail("Expected IllegalArgumentException for empty payload"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("must be between 1 and 64")); + } + // Invalid construction - payload too large try { new SignerKey.Ed25519SignedPayload(publicKey, invalidPayload); fail("Expected IllegalArgumentException for payload too large"); } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("Invalid payload length, must be less than 64")); + assertTrue(e.getMessage().contains("must be between 1 and 64")); } } @@ -273,4 +282,166 @@ public void testEqualsAndHashCode() { assertNotEquals(signerKey1, signerKey3); assertNotEquals(signerKey1.hashCode(), signerKey3.hashCode()); } + + @Test + public void testToEd25519SignedPayloadKeyTooShort() { + // Key length < 40 bytes + byte[] shortKey = new byte[39]; + SignerKey signerKey = + new SignerKey(shortKey, SignerKeyType.SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD); + + try { + signerKey.toEd25519SignedPayload(); + fail("Expected IllegalArgumentException for key too short"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("must be between 40 and 100 bytes")); + } + } + + @Test + public void testToEd25519SignedPayloadKeyTooLong() { + // Key length > 100 bytes + byte[] longKey = new byte[101]; + SignerKey signerKey = + new SignerKey(longKey, SignerKeyType.SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD); + + try { + signerKey.toEd25519SignedPayload(); + fail("Expected IllegalArgumentException for key too long"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("must be between 40 and 100 bytes")); + } + } + + @Test + public void testToEd25519SignedPayloadNegativePayloadLength() { + // 32 bytes public key + 4 bytes negative length + 4 bytes padding = 40 bytes + byte[] key = new byte[40]; + // Set payload length to -1 (0xFFFFFFFF in big-endian) + key[32] = (byte) 0xFF; + key[33] = (byte) 0xFF; + key[34] = (byte) 0xFF; + key[35] = (byte) 0xFF; + + SignerKey signerKey = new SignerKey(key, SignerKeyType.SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD); + + try { + signerKey.toEd25519SignedPayload(); + fail("Expected IllegalArgumentException for negative payload length"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("must be between 1 and 64")); + } + } + + @Test + public void testToEd25519SignedPayloadZeroPayloadLength() { + // 32 bytes public key + 4 bytes zero length + 4 bytes padding = 40 bytes + byte[] key = new byte[40]; + // payload length is already 0 (default) + + SignerKey signerKey = new SignerKey(key, SignerKeyType.SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD); + + try { + signerKey.toEd25519SignedPayload(); + fail("Expected IllegalArgumentException for zero payload length"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("must be between 1 and 64")); + } + } + + @Test + public void testToEd25519SignedPayloadPayloadLengthTooLarge() { + // 32 bytes public key + 4 bytes length (65) + padding = need more space + byte[] key = new byte[100]; + // Set payload length to 65 (0x00000041 in big-endian) + key[32] = 0; + key[33] = 0; + key[34] = 0; + key[35] = 65; + + SignerKey signerKey = new SignerKey(key, SignerKeyType.SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD); + + try { + signerKey.toEd25519SignedPayload(); + fail("Expected IllegalArgumentException for payload length > 64"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("must be between 1 and 64")); + } + } + + @Test + public void testToEd25519SignedPayloadLengthMismatch() { + // 32 bytes public key + 4 bytes length (1) + 1 byte payload + 3 bytes padding = 40 bytes + // But we provide 44 bytes total + byte[] key = new byte[44]; + // Set payload length to 1 + key[35] = 1; + + SignerKey signerKey = new SignerKey(key, SignerKeyType.SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD); + + try { + signerKey.toEd25519SignedPayload(); + fail("Expected IllegalArgumentException for length mismatch"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("expected 40 bytes")); + } + } + + @Test + public void testToEd25519SignedPayloadNonZeroPadding() { + // 32 bytes public key + 4 bytes length (1) + 1 byte payload + 3 bytes padding = 40 bytes + byte[] key = new byte[40]; + // Set payload length to 1 + key[35] = 1; + // Set payload byte + key[36] = 0x42; + // Set non-zero padding (should be zeros) + key[37] = 0x01; // This should be 0 + + SignerKey signerKey = new SignerKey(key, SignerKeyType.SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD); + + try { + signerKey.toEd25519SignedPayload(); + fail("Expected IllegalArgumentException for non-zero padding"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("padding bytes must be zero")); + } + } + + @Test + public void testToEd25519SignedPayloadValidMinPayload() { + // 32 bytes public key + 4 bytes length (1) + 1 byte payload + 3 bytes zero padding = 40 bytes + byte[] key = new byte[40]; + // Set payload length to 1 (big-endian) + key[35] = 1; + // Set payload byte + key[36] = 0x42; + // Padding bytes key[37], key[38], key[39] are already 0 + + SignerKey signerKey = new SignerKey(key, SignerKeyType.SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD); + SignerKey.Ed25519SignedPayload result = signerKey.toEd25519SignedPayload(); + + assertEquals(1, result.getPayload().length); + assertEquals(0x42, result.getPayload()[0]); + } + + @Test + public void testToEd25519SignedPayloadValidMaxPayload() { + // 32 bytes public key + 4 bytes length (64) + 64 bytes payload + 0 bytes padding = 100 bytes + byte[] key = new byte[100]; + // Set payload length to 64 (big-endian) + key[35] = 64; + // Fill payload with test data + for (int i = 0; i < 64; i++) { + key[36 + i] = (byte) i; + } + + SignerKey signerKey = new SignerKey(key, SignerKeyType.SIGNER_KEY_TYPE_ED25519_SIGNED_PAYLOAD); + SignerKey.Ed25519SignedPayload result = signerKey.toEd25519SignedPayload(); + + assertEquals(64, result.getPayload().length); + for (int i = 0; i < 64; i++) { + assertEquals((byte) i, result.getPayload()[i]); + } + } } diff --git a/src/test/kotlin/org/stellar/sdk/StrKeyTest.kt b/src/test/kotlin/org/stellar/sdk/StrKeyTest.kt index 8867ec0c0..e79da148a 100644 --- a/src/test/kotlin/org/stellar/sdk/StrKeyTest.kt +++ b/src/test/kotlin/org/stellar/sdk/StrKeyTest.kt @@ -275,7 +275,66 @@ class StrKeyTest : .message shouldBe "Invalid data length, the length should be between 40 and 100 bytes, got 39" } - // TODO: add encode check + + test("decode with zero payload length should fail") { + // 32 bytes public key + 4 bytes length (0) + 4 bytes padding = 40 bytes + // But payloadLength = 0 means padding calc gives 0, so actual = 36 bytes + // This will fail the minimum length check first + val rawKey = ByteArray(40) + // payload length is 0 (default), which should be rejected + val encoded = StrKey.encodeSignedPayload(rawKey) + shouldThrow { StrKey.decodeSignedPayload(encoded) } + .message shouldBe + "Invalid Ed25519 Signed Payload Key, payload length must be between 1 and 64, got 0" + } + + test("decode with payload length > 64 should fail") { + // Craft raw data with payload length = 65 + val rawKey = ByteArray(100) + rawKey[35] = 65 // Set payload length to 65 + val encoded = StrKey.encodeSignedPayload(rawKey) + shouldThrow { StrKey.decodeSignedPayload(encoded) } + .message shouldBe + "Invalid Ed25519 Signed Payload Key, payload length must be between 1 and 64, got 65" + } + + test("decode with non-zero padding should fail") { + // 32 bytes public key + 4 bytes length (1) + 1 byte payload + 3 bytes padding = 40 bytes + val rawKey = ByteArray(40) + rawKey[35] = 1 // payload length = 1 + rawKey[36] = 0x42 // payload byte + rawKey[37] = 0x01 // non-zero padding (should be 0) + val encoded = StrKey.encodeSignedPayload(rawKey) + shouldThrow { StrKey.decodeSignedPayload(encoded) } + .message shouldBe "Invalid Ed25519 Signed Payload Key, padding bytes must be zero" + } + + test("encode and decode with minimum valid payload (1 byte)") { + // 32 bytes public key + 4 bytes length (1) + 1 byte payload + 3 bytes zero padding = 40 + // bytes + val rawKey = ByteArray(40) + rawKey[35] = 1 // payload length = 1 + rawKey[36] = 0x42 // payload byte + // padding bytes [37], [38], [39] are already 0 + + val encoded = StrKey.encodeSignedPayload(rawKey) + val decoded = StrKey.decodeSignedPayload(encoded) + decoded shouldBe rawKey + } + + test("encode and decode with maximum valid payload (64 bytes)") { + // 32 bytes public key + 4 bytes length (64) + 64 bytes payload + 0 bytes padding = 100 + // bytes + val rawKey = ByteArray(100) + rawKey[35] = 64 // payload length = 64 + for (i in 0 until 64) { + rawKey[36 + i] = i.toByte() + } + + val encoded = StrKey.encodeSignedPayload(rawKey) + val decoded = StrKey.decodeSignedPayload(encoded) + decoded shouldBe rawKey + } } context("should reject all invalid StrKey cases") {