From d55a547740b28c834ba4ef4d115872995d280ae3 Mon Sep 17 00:00:00 2001 From: Andreas Schildbach Date: Sat, 3 Mar 2018 09:49:02 +0100 Subject: [PATCH] AddressFormatException: Add InvalidDataLength exception that is thrown when the data part isn't of the right size. --- .../org/bitcoinj/core/AddressFormatException.java | 15 +++++++++++++++ core/src/main/java/org/bitcoinj/core/Base58.java | 2 +- core/src/main/java/org/bitcoinj/core/Bech32.java | 13 ++++++++----- .../java/org/bitcoinj/core/DumpedPrivateKey.java | 3 ++- .../java/org/bitcoinj/core/LegacyAddress.java | 3 ++- .../java/org/bitcoinj/core/SegwitAddress.java | 7 ++++--- .../java/org/bitcoinj/crypto/BIP38PrivateKey.java | 2 +- .../test/java/org/bitcoinj/core/Base58Test.java | 2 +- .../org/bitcoinj/core/DumpedPrivateKeyTest.java | 12 ++++++++++++ .../java/org/bitcoinj/core/SegwitAddressTest.java | 15 +++++++++++++++ .../org/bitcoinj/crypto/BIP38PrivateKeyTest.java | 8 ++++++++ 11 files changed, 69 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/AddressFormatException.java b/core/src/main/java/org/bitcoinj/core/AddressFormatException.java index 88843c57c..c78460263 100644 --- a/core/src/main/java/org/bitcoinj/core/AddressFormatException.java +++ b/core/src/main/java/org/bitcoinj/core/AddressFormatException.java @@ -43,6 +43,21 @@ public class AddressFormatException extends IllegalArgumentException { } } + /** + * This exception is thrown by {@link Base58}, {@link Bech32} and the {@link PrefixedChecksummedBytes} hierarchy of + * classes when you try to decode data and the data isn't of the right size. You shouldn't allow the user to proceed + * in this case. + */ + public static class InvalidDataLength extends AddressFormatException { + public InvalidDataLength() { + super(); + } + + public InvalidDataLength(String message) { + super(message); + } + } + /** * This exception is thrown by {@link Base58}, {@link Bech32} and the {@link PrefixedChecksummedBytes} hierarchy of * classes when you try to decode data and the checksum isn't valid. You shouldn't allow the user to proceed in this diff --git a/core/src/main/java/org/bitcoinj/core/Base58.java b/core/src/main/java/org/bitcoinj/core/Base58.java index 8d94a205b..6ec2c47ac 100644 --- a/core/src/main/java/org/bitcoinj/core/Base58.java +++ b/core/src/main/java/org/bitcoinj/core/Base58.java @@ -170,7 +170,7 @@ public class Base58 { public static byte[] decodeChecked(String input) throws AddressFormatException { byte[] decoded = decode(input); if (decoded.length < 4) - throw new AddressFormatException("Input too short"); + throw new AddressFormatException.InvalidDataLength("Input too short: " + decoded.length); byte[] data = Arrays.copyOfRange(decoded, 0, decoded.length - 4); byte[] checksum = Arrays.copyOfRange(decoded, decoded.length - 4, decoded.length); byte[] actualChecksum = Arrays.copyOfRange(Sha256Hash.hashTwice(data), 0, 4); diff --git a/core/src/main/java/org/bitcoinj/core/Bech32.java b/core/src/main/java/org/bitcoinj/core/Bech32.java index 118f1ae62..c6cf72e75 100644 --- a/core/src/main/java/org/bitcoinj/core/Bech32.java +++ b/core/src/main/java/org/bitcoinj/core/Bech32.java @@ -124,8 +124,10 @@ public class Bech32 { /** Decode a Bech32 string. */ public static Bech32Data decode(final String str) throws AddressFormatException { boolean lower = false, upper = false; - if (str.length() < 8) throw new AddressFormatException("Input too short"); - if (str.length() > 90) throw new AddressFormatException("Input too long"); + if (str.length() < 8) + throw new AddressFormatException.InvalidDataLength("Input too short: " + str.length()); + if (str.length() > 90) + throw new AddressFormatException.InvalidDataLength("Input too long: " + str.length()); for (int i = 0; i < str.length(); ++i) { char c = str.charAt(i); if (c < 33 || c > 126) throw new AddressFormatException.InvalidCharacter(c, i); @@ -142,9 +144,10 @@ public class Bech32 { } final int pos = str.lastIndexOf('1'); if (pos < 1) throw new AddressFormatException("Missing human-readable part"); - if (pos + 7 > str.length()) throw new AddressFormatException("Data part too short"); - byte[] values = new byte[str.length() - 1 - pos]; - for (int i = 0; i < str.length() - 1 - pos; ++i) { + final int dataPartLength = str.length() - 1 - pos; + if (dataPartLength < 6) throw new AddressFormatException.InvalidDataLength("Data part too short: " + dataPartLength); + byte[] values = new byte[dataPartLength]; + for (int i = 0; i < dataPartLength; ++i) { char c = str.charAt(i + pos + 1); if (CHARSET_REV[c] == -1) throw new AddressFormatException.InvalidCharacter(c, i + pos + 1); values[i] = CHARSET_REV[c]; diff --git a/core/src/main/java/org/bitcoinj/core/DumpedPrivateKey.java b/core/src/main/java/org/bitcoinj/core/DumpedPrivateKey.java index 59b3afcb2..2268eb4a3 100644 --- a/core/src/main/java/org/bitcoinj/core/DumpedPrivateKey.java +++ b/core/src/main/java/org/bitcoinj/core/DumpedPrivateKey.java @@ -63,7 +63,8 @@ public class DumpedPrivateKey extends PrefixedChecksummedBytes { private DumpedPrivateKey(NetworkParameters params, byte[] bytes) { super(params, bytes); if (bytes.length != 32 && bytes.length != 33) - throw new AddressFormatException("Wrong number of bytes for a private key, not 32 or 33"); + throw new AddressFormatException.InvalidDataLength( + "Wrong number of bytes for a private key (32 or 33): " + bytes.length); } // Used by ECKey.getPrivateKeyEncoded() diff --git a/core/src/main/java/org/bitcoinj/core/LegacyAddress.java b/core/src/main/java/org/bitcoinj/core/LegacyAddress.java index ebf879a4b..a62578cab 100644 --- a/core/src/main/java/org/bitcoinj/core/LegacyAddress.java +++ b/core/src/main/java/org/bitcoinj/core/LegacyAddress.java @@ -66,7 +66,8 @@ public class LegacyAddress extends Address { private LegacyAddress(NetworkParameters params, boolean p2sh, byte[] hash160) throws AddressFormatException { super(params, hash160); if (hash160.length != 20) - throw new AddressFormatException("Legacy addresses are 160-bit hashes, so you must provide 20 bytes"); + throw new AddressFormatException.InvalidDataLength( + "Legacy addresses are 20 byte (160 bit) hashes, but got: " + hash160.length); this.p2sh = p2sh; } diff --git a/core/src/main/java/org/bitcoinj/core/SegwitAddress.java b/core/src/main/java/org/bitcoinj/core/SegwitAddress.java index 47654f6f3..f0efe25ac 100644 --- a/core/src/main/java/org/bitcoinj/core/SegwitAddress.java +++ b/core/src/main/java/org/bitcoinj/core/SegwitAddress.java @@ -89,17 +89,18 @@ public class SegwitAddress extends Address { private SegwitAddress(NetworkParameters params, byte[] data) throws AddressFormatException { super(params, data); if (data.length < 1) - throw new AddressFormatException("Zero data found"); + throw new AddressFormatException.InvalidDataLength("Zero data found"); final int witnessVersion = getWitnessVersion(); if (witnessVersion < 0 || witnessVersion > 16) throw new AddressFormatException("Invalid script version: " + witnessVersion); byte[] witnessProgram = getWitnessProgram(); if (witnessProgram.length < WITNESS_PROGRAM_MIN_LENGTH || witnessProgram.length > WITNESS_PROGRAM_MAX_LENGTH) - throw new AddressFormatException("Invalid length: " + witnessProgram.length); + throw new AddressFormatException.InvalidDataLength("Invalid length: " + witnessProgram.length); // Check script length for version 0 if (witnessVersion == 0 && witnessProgram.length != WITNESS_PROGRAM_LENGTH_PKH && witnessProgram.length != WITNESS_PROGRAM_LENGTH_SH) - throw new AddressFormatException("Invalid length for address version 0: " + witnessProgram.length); + throw new AddressFormatException.InvalidDataLength( + "Invalid length for address version 0: " + witnessProgram.length); } /** diff --git a/core/src/main/java/org/bitcoinj/crypto/BIP38PrivateKey.java b/core/src/main/java/org/bitcoinj/crypto/BIP38PrivateKey.java index d2c901337..904ed197d 100644 --- a/core/src/main/java/org/bitcoinj/crypto/BIP38PrivateKey.java +++ b/core/src/main/java/org/bitcoinj/crypto/BIP38PrivateKey.java @@ -62,7 +62,7 @@ public class BIP38PrivateKey extends PrefixedChecksummedBytes { if (version != 0x01) throw new AddressFormatException("Mismatched version number: " + version); if (bytes.length != 38) - throw new AddressFormatException("Wrong number of bytes, excluding version byte: " + bytes.length); + throw new AddressFormatException.InvalidDataLength("Wrong number of bytes: " + bytes.length); boolean hasLotAndSequence = (bytes[1] & 0x04) != 0; // bit 2 boolean compressed = (bytes[1] & 0x20) != 0; // bit 5 if ((bytes[1] & 0x01) != 0) // bit 0 diff --git a/core/src/test/java/org/bitcoinj/core/Base58Test.java b/core/src/test/java/org/bitcoinj/core/Base58Test.java index 63097f2d9..68c8b8175 100644 --- a/core/src/test/java/org/bitcoinj/core/Base58Test.java +++ b/core/src/test/java/org/bitcoinj/core/Base58Test.java @@ -93,7 +93,7 @@ public class Base58Test { Base58.decodeChecked("4stwEBjT6FYyVW"); } - @Test(expected = AddressFormatException.class) + @Test(expected = AddressFormatException.InvalidDataLength.class) public void testDecodeChecked_shortInput() { Base58.decodeChecked("4s"); } diff --git a/core/src/test/java/org/bitcoinj/core/DumpedPrivateKeyTest.java b/core/src/test/java/org/bitcoinj/core/DumpedPrivateKeyTest.java index 922594188..2c45d010a 100644 --- a/core/src/test/java/org/bitcoinj/core/DumpedPrivateKeyTest.java +++ b/core/src/test/java/org/bitcoinj/core/DumpedPrivateKeyTest.java @@ -92,6 +92,18 @@ public class DumpedPrivateKeyTest { DumpedPrivateKey.fromBase58(null, base58); // fail } + @Test(expected = AddressFormatException.InvalidDataLength.class) + public void fromBase58_tooShort() { + String base58 = Base58.encodeChecked(MAINNET.dumpedPrivateKeyHeader, new byte[31]); + DumpedPrivateKey.fromBase58(null, base58); + } + + @Test(expected = AddressFormatException.InvalidDataLength.class) + public void fromBase58_tooLong() { + String base58 = Base58.encodeChecked(MAINNET.dumpedPrivateKeyHeader, new byte[34]); + DumpedPrivateKey.fromBase58(null, base58); + } + @Test public void roundtripBase58_getKey() throws Exception { ECKey k = new ECKey().decompress(); diff --git a/core/src/test/java/org/bitcoinj/core/SegwitAddressTest.java b/core/src/test/java/org/bitcoinj/core/SegwitAddressTest.java index 66678e40a..41f0ffcf1 100644 --- a/core/src/test/java/org/bitcoinj/core/SegwitAddressTest.java +++ b/core/src/test/java/org/bitcoinj/core/SegwitAddressTest.java @@ -159,6 +159,21 @@ public class SegwitAddressTest { "bc1zw508d6qejxtdg4y5r3zarvaryvqyzf3du", "tb1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3pjxtptv", "bc1gmk9yu" }; + @Test(expected = AddressFormatException.InvalidDataLength.class) + public void fromBech32_version0_invalidLength() { + SegwitAddress.fromBech32(null, "BC1QR508D6QEJXTDG4Y5R3ZARVARYV98GJ9P"); + } + + @Test(expected = AddressFormatException.InvalidDataLength.class) + public void fromBech32_tooShort() { + SegwitAddress.fromBech32(null, "bc1rw5uspcuh"); + } + + @Test(expected = AddressFormatException.InvalidDataLength.class) + public void fromBech32_tooLong() { + SegwitAddress.fromBech32(null, "bc10w508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7kw5rljs90"); + } + @Test public void testJavaSerialization() throws Exception { SegwitAddress address = SegwitAddress.fromBech32(null, "BC1SW50QA3JX3S"); diff --git a/core/src/test/java/org/bitcoinj/crypto/BIP38PrivateKeyTest.java b/core/src/test/java/org/bitcoinj/crypto/BIP38PrivateKeyTest.java index 589b044cf..f28f77eb7 100644 --- a/core/src/test/java/org/bitcoinj/crypto/BIP38PrivateKeyTest.java +++ b/core/src/test/java/org/bitcoinj/crypto/BIP38PrivateKeyTest.java @@ -16,6 +16,8 @@ package org.bitcoinj.crypto; +import org.bitcoinj.core.AddressFormatException; +import org.bitcoinj.core.Base58; import org.bitcoinj.core.ECKey; import org.bitcoinj.core.NetworkParameters; import org.bitcoinj.crypto.BIP38PrivateKey.BadPassphraseException; @@ -149,6 +151,12 @@ public class BIP38PrivateKeyTest { encryptedKey.decrypt("BAD"); } + @Test(expected = AddressFormatException.InvalidDataLength.class) + public void fromBase58_invalidLength() { + String base58 = Base58.encodeChecked(1, new byte[16]); + BIP38PrivateKey.fromBase58(null, base58); + } + @Test public void testJavaSerialization() throws Exception { BIP38PrivateKey testKey = BIP38PrivateKey.fromBase58(TESTNET,