diff --git a/gradle.properties b/gradle.properties index 35de375e4f..c578d00186 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,5 +1,5 @@ org.gradle.jvmargs=-Xmx2g -version=1.81 +version=1.82-SNAPSHOT maxVersion=1.82 org.gradle.java.installations.auto-detect=false org.gradle.java.installations.auto-download=false diff --git a/pg/src/main/java/org/bouncycastle/bcpg/AEADEncDataPacket.java b/pg/src/main/java/org/bouncycastle/bcpg/AEADEncDataPacket.java index f417e94d70..c3fada791a 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/AEADEncDataPacket.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/AEADEncDataPacket.java @@ -40,14 +40,22 @@ public AEADEncDataPacket(BCPGInputStream in, version = (byte)in.read(); if (version != VERSION_1) { - throw new UnsupportedPacketVersionException("wrong AEAD packet version: " + version); + throw new UnsupportedPacketVersionException("Unknown AEAD packet version: " + version); } algorithm = (byte)in.read(); aeadAlgorithm = (byte)in.read(); chunkSize = (byte)in.read(); - iv = new byte[AEADUtils.getIVLength(aeadAlgorithm)]; + try + { + int ivLen = AEADUtils.getIVLength(aeadAlgorithm); + iv = new byte[ivLen]; + } + catch (IllegalArgumentException e) + { + throw new MalformedPacketException("Unknown AEAD algorithm ID: " + aeadAlgorithm, e); + } in.readFully(iv); } diff --git a/pg/src/main/java/org/bouncycastle/bcpg/ECDHPublicBCPGKey.java b/pg/src/main/java/org/bouncycastle/bcpg/ECDHPublicBCPGKey.java index 807459052a..efb6419ed2 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/ECDHPublicBCPGKey.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/ECDHPublicBCPGKey.java @@ -36,12 +36,11 @@ public ECDHPublicBCPGKey( super(in); int length = in.read(); - byte[] kdfParameters = new byte[length]; - if (kdfParameters.length != 3) + if (length != 3) { - throw new IllegalStateException("kdf parameters size of 3 expected."); + throw new MalformedPacketException("KDF parameters size of 3 expected."); } - + byte[] kdfParameters = new byte[length]; in.readFully(kdfParameters); reserved = kdfParameters[0]; diff --git a/pg/src/main/java/org/bouncycastle/bcpg/LiteralDataPacket.java b/pg/src/main/java/org/bouncycastle/bcpg/LiteralDataPacket.java index 2b1a196a51..8ab7ad86af 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/LiteralDataPacket.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/LiteralDataPacket.java @@ -31,6 +31,10 @@ public class LiteralDataPacket format = in.read(); int l = in.read(); + if (l < 0) + { + throw new MalformedPacketException("File name size cannot be negative."); + } fileName = new byte[l]; for (int i = 0; i != fileName.length; i++) diff --git a/pg/src/main/java/org/bouncycastle/bcpg/MalformedPacketException.java b/pg/src/main/java/org/bouncycastle/bcpg/MalformedPacketException.java new file mode 100644 index 0000000000..784a6253f6 --- /dev/null +++ b/pg/src/main/java/org/bouncycastle/bcpg/MalformedPacketException.java @@ -0,0 +1,23 @@ +package org.bouncycastle.bcpg; + +import java.io.IOException; + +public class MalformedPacketException + extends IOException +{ + + public MalformedPacketException(String message) + { + super(message); + } + + public MalformedPacketException(Throwable cause) + { + super(cause); + } + + public MalformedPacketException(String message, Throwable cause) + { + super(message, cause); + } +} diff --git a/pg/src/main/java/org/bouncycastle/bcpg/OctetArrayBCPGKey.java b/pg/src/main/java/org/bouncycastle/bcpg/OctetArrayBCPGKey.java index 483d41729b..f846480fbf 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/OctetArrayBCPGKey.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/OctetArrayBCPGKey.java @@ -3,6 +3,8 @@ import java.io.IOException; import org.bouncycastle.util.Arrays; +import static org.bouncycastle.bcpg.PublicKeyPacket.MAX_LEN; + /** * Public/Secret BCPGKey which is encoded as an array of octets rather than an MPI. */ @@ -15,6 +17,10 @@ public abstract class OctetArrayBCPGKey OctetArrayBCPGKey(int length, BCPGInputStream in) throws IOException { + if (length > MAX_LEN) + { + throw new IOException("Max key length (" + MAX_LEN + ") exceeded (" + length + ")"); + } key = new byte[length]; in.readFully(key); } diff --git a/pg/src/main/java/org/bouncycastle/bcpg/Packet.java b/pg/src/main/java/org/bouncycastle/bcpg/Packet.java index 8456713607..237da7c748 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/Packet.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/Packet.java @@ -60,4 +60,18 @@ public boolean isCritical() { return getPacketTag() <= 39; } + + static int sanitizeLength(int len, int max, String variableName) + throws MalformedPacketException + { + if (len < 0) + { + throw new MalformedPacketException(variableName + " cannot be negative."); + } + if (len > max) + { + throw new MalformedPacketException(variableName + " (" + len + ") exceeds limit (" + max + ")."); + } + return len; + } } diff --git a/pg/src/main/java/org/bouncycastle/bcpg/PublicKeyEncSessionPacket.java b/pg/src/main/java/org/bouncycastle/bcpg/PublicKeyEncSessionPacket.java index d7a6e4ec52..b629441fb8 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/PublicKeyEncSessionPacket.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/PublicKeyEncSessionPacket.java @@ -55,6 +55,10 @@ public class PublicKeyEncSessionPacket else if (version == VERSION_6) { int keyInfoLen = in.read(); + if (keyInfoLen < 0) + { + throw new MalformedPacketException("Key Info Length cannot be negative: " + keyInfoLen); + } if (keyInfoLen == 0) { // anon recipient @@ -69,13 +73,20 @@ else if (version == VERSION_6) in.readFully(keyFingerprint); // Derived key-ID from fingerprint // TODO: Replace with getKeyIdentifier - if (keyVersion == PublicKeyPacket.VERSION_4) + try { - keyID = FingerprintUtil.keyIdFromV4Fingerprint(keyFingerprint); + if (keyVersion == PublicKeyPacket.VERSION_4) + { + keyID = FingerprintUtil.keyIdFromV4Fingerprint(keyFingerprint); + } + else + { + keyID = FingerprintUtil.keyIdFromV6Fingerprint(keyFingerprint); + } } - else + catch (IllegalArgumentException e) { - keyID = FingerprintUtil.keyIdFromV6Fingerprint(keyFingerprint); + throw new MalformedPacketException("Malformed fingerprint encoding.", e); } } } diff --git a/pg/src/main/java/org/bouncycastle/bcpg/PublicKeyPacket.java b/pg/src/main/java/org/bouncycastle/bcpg/PublicKeyPacket.java index b8a4fa0b25..4ae287f26e 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/PublicKeyPacket.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/PublicKeyPacket.java @@ -22,6 +22,8 @@ public class PublicKeyPacket extends ContainedPacket implements PublicKeyAlgorithmTags { + public static int MAX_LEN = 2 * 1024 * 1024; // 2mb; McEliece keys can get ~1mb in size, so allow some margin + /** * OpenPGP v3 keys are deprecated. * They can only be used with RSA. @@ -150,6 +152,10 @@ public class PublicKeyPacket { // TODO: Use keyOctets to be able to parse unknown keys keyOctets = StreamUtil.read4OctetLength(in); + if (keyOctets < 0) + { + throw new MalformedPacketException("Octet length cannot be negative."); + } } parseKey(in, algorithm, keyOctets); @@ -165,50 +171,56 @@ public class PublicKeyPacket private void parseKey(BCPGInputStream in, int algorithmId, long optLen) throws IOException { - - switch (algorithmId) + try { - case RSA_ENCRYPT: - case RSA_GENERAL: - case RSA_SIGN: - key = new RSAPublicBCPGKey(in); - break; - case DSA: - key = new DSAPublicBCPGKey(in); - break; - case ELGAMAL_ENCRYPT: - case ELGAMAL_GENERAL: - key = new ElGamalPublicBCPGKey(in); - break; - case ECDH: - key = new ECDHPublicBCPGKey(in); - break; - case X25519: - key = new X25519PublicBCPGKey(in); - break; - case X448: - key = new X448PublicBCPGKey(in); - break; - case ECDSA: - key = new ECDSAPublicBCPGKey(in); - break; - case EDDSA_LEGACY: - key = new EdDSAPublicBCPGKey(in); - break; - case Ed25519: - key = new Ed25519PublicBCPGKey(in); - break; - case Ed448: - key = new Ed448PublicBCPGKey(in); - break; - default: - if (version == VERSION_6 || version == LIBREPGP_5) + switch (algorithmId) { - // with version 5 & 6, we can gracefully handle unknown key types, as the length is known. - key = new UnknownBCPGKey((int) optLen, in); + case RSA_ENCRYPT: + case RSA_GENERAL: + case RSA_SIGN: + key = new RSAPublicBCPGKey(in); + break; + case DSA: + key = new DSAPublicBCPGKey(in); + break; + case ELGAMAL_ENCRYPT: + case ELGAMAL_GENERAL: + key = new ElGamalPublicBCPGKey(in); + break; + case ECDH: + key = new ECDHPublicBCPGKey(in); + break; + case X25519: + key = new X25519PublicBCPGKey(in); break; + case X448: + key = new X448PublicBCPGKey(in); + break; + case ECDSA: + key = new ECDSAPublicBCPGKey(in); + break; + case EDDSA_LEGACY: + key = new EdDSAPublicBCPGKey(in); + break; + case Ed25519: + key = new Ed25519PublicBCPGKey(in); + break; + case Ed448: + key = new Ed448PublicBCPGKey(in); + break; + default: + if (version == VERSION_6 || version == LIBREPGP_5) + { + // with version 5 & 6, we can gracefully handle unknown key types, as the length is known. + key = new UnknownBCPGKey((int) optLen, in); + break; + } + throw new IOException("unknown PGP public key algorithm encountered: " + algorithm); } - throw new IOException("unknown PGP public key algorithm encountered: " + algorithm); + } + catch (IllegalStateException e) + { + throw new MalformedPacketException("Malformed PGP key.", e); } } diff --git a/pg/src/main/java/org/bouncycastle/bcpg/SecretKeyPacket.java b/pg/src/main/java/org/bouncycastle/bcpg/SecretKeyPacket.java index 9b59fd4f5c..9f1cca85e7 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/SecretKeyPacket.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/SecretKeyPacket.java @@ -7,6 +7,8 @@ import org.bouncycastle.util.Arrays; import org.bouncycastle.util.io.Streams; +import static org.bouncycastle.bcpg.PublicKeyPacket.MAX_LEN; + /** * Base class for OpenPGP secret (primary) keys. */ @@ -14,6 +16,8 @@ public class SecretKeyPacket extends ContainedPacket implements PublicKeyAlgorithmTags { + public static int MAX_S2K_ENCODING_LEN = 8192; // arbitrary + /** * S2K-usage octet indicating that the secret key material is unprotected. */ @@ -179,6 +183,7 @@ public class SecretKeyPacket if (version == PublicKeyPacket.VERSION_6 && (s2kUsage == USAGE_SHA1 || s2kUsage == USAGE_AEAD)) { int s2KLen = in.read(); + s2KLen = sanitizeLength(s2KLen, MAX_S2K_ENCODING_LEN, "S2K length octet"); byte[] s2kBytes = new byte[s2KLen]; in.readFully(s2kBytes); @@ -194,7 +199,14 @@ public class SecretKeyPacket } if (s2kUsage == USAGE_AEAD) { - iv = new byte[AEADUtils.getIVLength(aeadAlgorithm)]; + try + { + iv = new byte[AEADUtils.getIVLength(aeadAlgorithm)]; + } + catch (IllegalArgumentException e) + { + throw new MalformedPacketException("Unknown AEAD algorithm", e); + } Streams.readFully(in, iv); } else @@ -227,6 +239,7 @@ public class SecretKeyPacket // encoded keyOctetCount does not contain checksum keyOctetCount += 2; } + keyOctetCount = sanitizeLength((int) keyOctetCount, MAX_LEN, "Key octet count"); this.secKeyData = new byte[(int) keyOctetCount]; in.readFully(secKeyData); } diff --git a/pg/src/main/java/org/bouncycastle/bcpg/SignaturePacket.java b/pg/src/main/java/org/bouncycastle/bcpg/SignaturePacket.java index 853a22090f..aafe1aae37 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/SignaturePacket.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/SignaturePacket.java @@ -18,6 +18,8 @@ public class SignaturePacket extends ContainedPacket implements PublicKeyAlgorithmTags { + public static int MAX_SUBPACKET_LEN = 2 * 1024 * 1024; // 2mb, allows for embedded McEliece keys for example. + public static final int VERSION_2 = 2; public static final int VERSION_3 = 3; public static final int VERSION_4 = 4; // https://datatracker.ietf.org/doc/rfc4880/ @@ -149,6 +151,10 @@ private void parseV6(BCPGInputStream in) in.readFully(fingerPrint); int saltSize = in.read(); + if (saltSize < 0) + { + throw new MalformedPacketException("Negative salt size."); + } salt = new byte[saltSize]; in.readFully(salt); @@ -174,11 +180,11 @@ private void parseSubpackets(BCPGInputStream in) SignatureSubpacket p = (SignatureSubpacket)vec.elementAt(i); if (p instanceof IssuerKeyID) { - keyID = ((IssuerKeyID)p).getKeyID(); + keyID = parseKeyIdOrThrow((IssuerKeyID) p); } else if (p instanceof SignatureCreationTime) { - creationTime = ((SignatureCreationTime)p).getTime().getTime(); + creationTime = parseCreationTimeOrThrow((SignatureCreationTime) p); } hashedData[i] = p; @@ -192,7 +198,7 @@ else if (p instanceof SignatureCreationTime) SignatureSubpacket p = (SignatureSubpacket)vec.elementAt(i); if (p instanceof IssuerKeyID) { - keyID = ((IssuerKeyID)p).getKeyID(); + keyID = parseKeyIdOrThrow((IssuerKeyID) p); } unhashedData[i] = p; @@ -202,6 +208,45 @@ else if (p instanceof SignatureCreationTime) setCreationTime(); } + private long parseKeyIdOrThrow(IssuerKeyID keyID) + throws MalformedPacketException + { + try + { + return keyID.getKeyID(); + } + catch (IllegalArgumentException e) + { + throw new MalformedPacketException("Malformed IssuerKeyID subpacket.", e); + } + } + + private long parseKeyIdOrThrow(IssuerFingerprint fingerprint) + throws MalformedPacketException + { + try + { + return fingerprint.getKeyID(); + } + catch (IllegalArgumentException e) + { + throw new MalformedPacketException("Malformed IssuerFingerprint subpacket.", e); + } + } + + private long parseCreationTimeOrThrow(SignatureCreationTime creationTime) + throws MalformedPacketException + { + try + { + return creationTime.getTime().getTime(); + } + catch (IllegalStateException e) + { + throw new MalformedPacketException("Malformed SignatureCreationTime subpacket.", e); + } + } + private Vector readSignatureSubpacketVector(BCPGInputStream in) throws IOException { @@ -214,6 +259,14 @@ private Vector readSignatureSubpacketVector(BCPGInputStream { hashedLength = StreamUtil.read2OctetLength(in); } + if (hashedLength < 0) + { + throw new MalformedPacketException("Signature subpackets encoding length cannot be negative."); + } + if (hashedLength > MAX_SUBPACKET_LEN) + { + throw new MalformedPacketException("Signature subpackets encoding length (" + hashedLength + ") exceeds max limit (" + MAX_SUBPACKET_LEN + ")"); + } byte[] hashed = new byte[hashedLength]; in.readFully(hashed); @@ -743,6 +796,7 @@ private void setCreationTime() * Therefore, we can also check the unhashed signature subpacket area. */ private void setIssuerKeyId() + throws MalformedPacketException { if (keyID != 0L) { @@ -754,12 +808,12 @@ private void setIssuerKeyId() SignatureSubpacket p = hashedData[idx]; if (p instanceof IssuerKeyID) { - keyID = ((IssuerKeyID) p).getKeyID(); + keyID = parseKeyIdOrThrow((IssuerKeyID) p); return; } if (p instanceof IssuerFingerprint) { - keyID = ((IssuerFingerprint) p).getKeyID(); + keyID = parseKeyIdOrThrow((IssuerFingerprint) p); return; } } @@ -769,12 +823,12 @@ private void setIssuerKeyId() SignatureSubpacket p = unhashedData[idx]; if (p instanceof IssuerKeyID) { - keyID = ((IssuerKeyID) p).getKeyID(); + keyID = parseKeyIdOrThrow((IssuerKeyID) p); return; } if (p instanceof IssuerFingerprint) { - keyID = ((IssuerFingerprint) p).getKeyID(); + keyID = parseKeyIdOrThrow((IssuerFingerprint) p); return; } } diff --git a/pg/src/main/java/org/bouncycastle/bcpg/SignatureSubpacketInputStream.java b/pg/src/main/java/org/bouncycastle/bcpg/SignatureSubpacketInputStream.java index b66ba0813c..12be2a7e70 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/SignatureSubpacketInputStream.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/SignatureSubpacketInputStream.java @@ -128,58 +128,65 @@ else if (flags[StreamUtil.flag_partial]) } } - switch (type) + try { - case CREATION_TIME: - return new SignatureCreationTime(isCritical, isLongLength, data); - case EMBEDDED_SIGNATURE: - return new EmbeddedSignature(isCritical, isLongLength, data); - case KEY_EXPIRE_TIME: - return new KeyExpirationTime(isCritical, isLongLength, data); - case EXPIRE_TIME: - return new SignatureExpirationTime(isCritical, isLongLength, data); - case REVOCABLE: - return new Revocable(isCritical, isLongLength, data); - case EXPORTABLE: - return new Exportable(isCritical, isLongLength, data); - case FEATURES: - return new Features(isCritical, isLongLength, data); - case ISSUER_KEY_ID: - return new IssuerKeyID(isCritical, isLongLength, data); - case TRUST_SIG: - return new TrustSignature(isCritical, isLongLength, data); - case PREFERRED_COMP_ALGS: - case PREFERRED_HASH_ALGS: - case PREFERRED_SYM_ALGS: - return new PreferredAlgorithms(type, isCritical, isLongLength, data); - case LIBREPGP_PREFERRED_ENCRYPTION_MODES: - return new LibrePGPPreferredEncryptionModes(isCritical, isLongLength, data); - case PREFERRED_AEAD_ALGORITHMS: - return new PreferredAEADCiphersuites(isCritical, isLongLength, data); - case PREFERRED_KEY_SERV: - return new PreferredKeyServer(isCritical, isLongLength, data); - case KEY_FLAGS: - return new KeyFlags(isCritical, isLongLength, data); - case POLICY_URL: - return new PolicyURI(isCritical, isLongLength, data); - case PRIMARY_USER_ID: - return new PrimaryUserID(isCritical, isLongLength, data); - case SIGNER_USER_ID: - return new SignerUserID(isCritical, isLongLength, data); - case NOTATION_DATA: - return new NotationData(isCritical, isLongLength, data); - case REG_EXP: - return new RegularExpression(isCritical, isLongLength, data); - case REVOCATION_REASON: - return new RevocationReason(isCritical, isLongLength, data); - case REVOCATION_KEY: - return new RevocationKey(isCritical, isLongLength, data); - case SIGNATURE_TARGET: - return new SignatureTarget(isCritical, isLongLength, data); - case ISSUER_FINGERPRINT: - return new IssuerFingerprint(isCritical, isLongLength, data); - case INTENDED_RECIPIENT_FINGERPRINT: - return new IntendedRecipientFingerprint(isCritical, isLongLength, data); + switch (type) + { + case CREATION_TIME: + return new SignatureCreationTime(isCritical, isLongLength, data); + case EMBEDDED_SIGNATURE: + return new EmbeddedSignature(isCritical, isLongLength, data); + case KEY_EXPIRE_TIME: + return new KeyExpirationTime(isCritical, isLongLength, data); + case EXPIRE_TIME: + return new SignatureExpirationTime(isCritical, isLongLength, data); + case REVOCABLE: + return new Revocable(isCritical, isLongLength, data); + case EXPORTABLE: + return new Exportable(isCritical, isLongLength, data); + case FEATURES: + return new Features(isCritical, isLongLength, data); + case ISSUER_KEY_ID: + return new IssuerKeyID(isCritical, isLongLength, data); + case TRUST_SIG: + return new TrustSignature(isCritical, isLongLength, data); + case PREFERRED_COMP_ALGS: + case PREFERRED_HASH_ALGS: + case PREFERRED_SYM_ALGS: + return new PreferredAlgorithms(type, isCritical, isLongLength, data); + case LIBREPGP_PREFERRED_ENCRYPTION_MODES: + return new LibrePGPPreferredEncryptionModes(isCritical, isLongLength, data); + case PREFERRED_AEAD_ALGORITHMS: + return new PreferredAEADCiphersuites(isCritical, isLongLength, data); + case PREFERRED_KEY_SERV: + return new PreferredKeyServer(isCritical, isLongLength, data); + case KEY_FLAGS: + return new KeyFlags(isCritical, isLongLength, data); + case POLICY_URL: + return new PolicyURI(isCritical, isLongLength, data); + case PRIMARY_USER_ID: + return new PrimaryUserID(isCritical, isLongLength, data); + case SIGNER_USER_ID: + return new SignerUserID(isCritical, isLongLength, data); + case NOTATION_DATA: + return new NotationData(isCritical, isLongLength, data); + case REG_EXP: + return new RegularExpression(isCritical, isLongLength, data); + case REVOCATION_REASON: + return new RevocationReason(isCritical, isLongLength, data); + case REVOCATION_KEY: + return new RevocationKey(isCritical, isLongLength, data); + case SIGNATURE_TARGET: + return new SignatureTarget(isCritical, isLongLength, data); + case ISSUER_FINGERPRINT: + return new IssuerFingerprint(isCritical, isLongLength, data); + case INTENDED_RECIPIENT_FINGERPRINT: + return new IntendedRecipientFingerprint(isCritical, isLongLength, data); + } + } + catch (IllegalArgumentException e) + { + throw new MalformedPacketException("Malformed signature subpacket.", e); } return new SignatureSubpacket(type, isCritical, isLongLength, data); diff --git a/pg/src/main/java/org/bouncycastle/bcpg/SymmetricKeyEncSessionPacket.java b/pg/src/main/java/org/bouncycastle/bcpg/SymmetricKeyEncSessionPacket.java index aef46b6f2a..d8300c87b5 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/SymmetricKeyEncSessionPacket.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/SymmetricKeyEncSessionPacket.java @@ -81,7 +81,19 @@ else if (version == VERSION_5 || version == VERSION_6) } else { - ivLen = AEADUtils.getIVLength(aeadAlgorithm); + try + { + ivLen = AEADUtils.getIVLength(aeadAlgorithm); + } + catch (IllegalArgumentException e) + { + throw new IOException(e.getMessage(), e); + } + } + + if (ivLen < 0) + { + throw new MalformedPacketException("IV length cannot be negative."); } s2k = new S2K(in); @@ -92,11 +104,23 @@ else if (version == VERSION_5 || version == VERSION_6) throw new EOFException("Premature end of stream."); } - int authTagLen = AEADUtils.getAuthTagLength(aeadAlgorithm); + int authTagLen; + try + { + authTagLen = AEADUtils.getAuthTagLength(aeadAlgorithm); + } + catch (IllegalArgumentException e) + { + throw new MalformedPacketException("Unknown AEAD algorithm.", e); + } authTag = new byte[authTagLen]; // Read all trailing bytes byte[] sessKeyAndAuthTag = in.readAll(); + if (sessKeyAndAuthTag.length - authTagLen < 0) + { + throw new MalformedPacketException("AuthTagLen exceeds session key data."); + } // determine session key length by subtracting auth tag this.secKeyData = new byte[sessKeyAndAuthTag.length - authTagLen]; diff --git a/pg/src/main/java/org/bouncycastle/bcpg/UserAttributeSubpacketInputStream.java b/pg/src/main/java/org/bouncycastle/bcpg/UserAttributeSubpacketInputStream.java index dfc33988f6..590d9af10e 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/UserAttributeSubpacketInputStream.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/UserAttributeSubpacketInputStream.java @@ -14,11 +14,20 @@ public class UserAttributeSubpacketInputStream implements UserAttributeSubpacketTags { InputStream in; + private final int limit; public UserAttributeSubpacketInputStream( InputStream in) { + this(in, StreamUtil.findLimit(in)); + } + + public UserAttributeSubpacketInputStream( + InputStream in, + int limit + ) { this.in = in; + this.limit = limit; } public int available() @@ -72,13 +81,20 @@ public UserAttributeSubpacket readPacket() { boolean[] flags = new boolean[3]; int bodyLen = StreamUtil.readBodyLen(this, flags); + if (bodyLen < 1) { + throw new MalformedPacketException("Body length octet too small."); + } + if (bodyLen > limit) + { + throw new MalformedPacketException("Body length octet (" + bodyLen + ") exceeds limitations (" + limit + ")."); + } if (flags[StreamUtil.flag_eof]) { return null; } else if (flags[StreamUtil.flag_partial]) { - throw new IOException("unrecognised length reading user attribute sub packet"); + throw new MalformedPacketException("unrecognised length reading user attribute sub packet"); } boolean longLength = flags[StreamUtil.flag_isLongLength]; @@ -95,10 +111,17 @@ else if (flags[StreamUtil.flag_partial]) int type = tag; - switch (type) + try + { + switch (type) + { + case IMAGE_ATTRIBUTE: + return new ImageAttribute(longLength, data); + } + } + catch (IllegalArgumentException e) { - case IMAGE_ATTRIBUTE: - return new ImageAttribute(longLength, data); + throw new MalformedPacketException("Malformed UserAttribute subpacket.", e); } return new UserAttributeSubpacket(type, longLength, data); diff --git a/pg/src/main/java/org/bouncycastle/bcpg/attr/ImageAttribute.java b/pg/src/main/java/org/bouncycastle/bcpg/attr/ImageAttribute.java index 437d33ba42..9c8e8c6419 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/attr/ImageAttribute.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/attr/ImageAttribute.java @@ -37,8 +37,16 @@ public ImageAttribute( byte[] data) { super(UserAttributeSubpacketTags.IMAGE_ATTRIBUTE, forceLongLength, data); + if (data.length < 4) + { + throw new IllegalArgumentException("Malformed ImageAttribute. Data length too short: " + data.length); + } hdrLength = ((data[1] & 0xff) << 8) | (data[0] & 0xff); + if (data.length < hdrLength) + { + throw new IllegalArgumentException("Malformed ImageAttribute. Header length exceeds data length."); + } version = data[2] & 0xff; encoding = data[3] & 0xff; diff --git a/pg/src/main/java/org/bouncycastle/bcpg/sig/IntendedRecipientFingerprint.java b/pg/src/main/java/org/bouncycastle/bcpg/sig/IntendedRecipientFingerprint.java index dfc3d2c34d..44b4cd730a 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/sig/IntendedRecipientFingerprint.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/sig/IntendedRecipientFingerprint.java @@ -20,7 +20,7 @@ public IntendedRecipientFingerprint( boolean isLongLength, byte[] data) { - super(SignatureSubpacketTags.INTENDED_RECIPIENT_FINGERPRINT, critical, isLongLength, data); + super(SignatureSubpacketTags.INTENDED_RECIPIENT_FINGERPRINT, critical, isLongLength, verifyData(data)); } public IntendedRecipientFingerprint( @@ -32,6 +32,15 @@ public IntendedRecipientFingerprint( Arrays.prepend(fingerprint, (byte)keyVersion)); } + private static byte[] verifyData(byte[] data) + { + if (data.length < 1) + { + throw new IllegalArgumentException("Data too short. Expect at least one octet of key version number."); + } + return data; + } + public int getKeyVersion() { return data[0] & 0xff; diff --git a/pg/src/main/java/org/bouncycastle/bcpg/sig/IssuerFingerprint.java b/pg/src/main/java/org/bouncycastle/bcpg/sig/IssuerFingerprint.java index 7837c93a77..e79218bc0e 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/sig/IssuerFingerprint.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/sig/IssuerFingerprint.java @@ -22,7 +22,7 @@ public IssuerFingerprint( boolean isLongLength, byte[] data) { - super(SignatureSubpacketTags.ISSUER_FINGERPRINT, critical, isLongLength, data); + super(SignatureSubpacketTags.ISSUER_FINGERPRINT, critical, isLongLength, verifyData(data)); } public IssuerFingerprint( @@ -34,6 +34,15 @@ public IssuerFingerprint( Arrays.prepend(fingerprint, (byte)keyVersion)); } + private static byte[] verifyData(byte[] data) + { + if (data.length < 1) + { + throw new IllegalArgumentException("Data too short. Expect at least one octet of key version."); + } + return data; + } + public int getKeyVersion() { return data[0] & 0xff; diff --git a/pg/src/main/java/org/bouncycastle/bcpg/sig/NotationData.java b/pg/src/main/java/org/bouncycastle/bcpg/sig/NotationData.java index b1c4e970d7..226b9ddd26 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/sig/NotationData.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/sig/NotationData.java @@ -27,7 +27,7 @@ public NotationData( boolean isLongLength, byte[] data) { - super(SignatureSubpacketTags.NOTATION_DATA, critical, isLongLength, data); + super(SignatureSubpacketTags.NOTATION_DATA, critical, isLongLength, verifyData(data)); } public NotationData( @@ -39,6 +39,21 @@ public NotationData( super(SignatureSubpacketTags.NOTATION_DATA, critical, false, createData(humanReadable, notationName, notationValue)); } + private static byte[] verifyData(byte[] data) + { + if (data.length < 8) + { + throw new IllegalArgumentException("Malformed notation data encoding (too short): " + data.length); + } + int nameLength = (((data[HEADER_FLAG_LENGTH] & 0xff) << 8) + (data[HEADER_FLAG_LENGTH + 1] & 0xff)); + int valueLength = (((data[HEADER_FLAG_LENGTH + HEADER_NAME_LENGTH] & 0xff) << 8) + (data[HEADER_FLAG_LENGTH + HEADER_NAME_LENGTH + 1] & 0xff)); + if (nameLength + valueLength + 4 > data.length) + { + throw new IllegalArgumentException("Malformed notation data encoding."); + } + return data; + } + private static byte[] createData(boolean humanReadable, String notationName, String notationValue) { ByteArrayOutputStream out = new ByteArrayOutputStream(); diff --git a/pg/src/main/java/org/bouncycastle/bcpg/sig/RegularExpression.java b/pg/src/main/java/org/bouncycastle/bcpg/sig/RegularExpression.java index c780011a8a..2cafb9fbd4 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/sig/RegularExpression.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/sig/RegularExpression.java @@ -23,7 +23,7 @@ public RegularExpression( byte[] data) { super(SignatureSubpacketTags.REG_EXP, critical, isLongLength, data); - if (data[data.length - 1] != 0) + if (data.length == 0 || data[data.length - 1] != 0) { throw new IllegalArgumentException("data in regex missing null termination"); } diff --git a/pg/src/main/java/org/bouncycastle/bcpg/sig/SignatureExpirationTime.java b/pg/src/main/java/org/bouncycastle/bcpg/sig/SignatureExpirationTime.java index d89d86c10e..6d2bdc096f 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/sig/SignatureExpirationTime.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/sig/SignatureExpirationTime.java @@ -29,7 +29,7 @@ public SignatureExpirationTime( boolean isLongLength, byte[] data) { - super(SignatureSubpacketTags.EXPIRE_TIME, critical, isLongLength, data); + super(SignatureSubpacketTags.EXPIRE_TIME, critical, isLongLength, verifyData(data)); } public SignatureExpirationTime( @@ -39,6 +39,15 @@ public SignatureExpirationTime( super(SignatureSubpacketTags.EXPIRE_TIME, critical, false, Utils.timeToBytes(seconds)); } + private static byte[] verifyData(byte[] data) + { + if (data.length != 4) + { + throw new IllegalArgumentException("Malformed data length. Expected 4, got " + data.length); + } + return data; + } + /** * return time in seconds before signature expires after creation time. */ diff --git a/pg/src/main/java/org/bouncycastle/openpgp/PGPObjectFactory.java b/pg/src/main/java/org/bouncycastle/openpgp/PGPObjectFactory.java index 852fc2d0f1..6ea007d11b 100644 --- a/pg/src/main/java/org/bouncycastle/openpgp/PGPObjectFactory.java +++ b/pg/src/main/java/org/bouncycastle/openpgp/PGPObjectFactory.java @@ -9,6 +9,7 @@ import java.util.NoSuchElementException; import org.bouncycastle.bcpg.BCPGInputStream; +import org.bouncycastle.bcpg.MalformedPacketException; import org.bouncycastle.bcpg.PacketTags; import org.bouncycastle.bcpg.UnknownPacket; import org.bouncycastle.bcpg.UnsupportedPacketVersionException; @@ -146,7 +147,14 @@ public Object nextObject() case PacketTags.SYMMETRIC_KEY_ENC: case PacketTags.SYM_ENC_INTEGRITY_PRO: case PacketTags.AEAD_ENC_DATA: - return new PGPEncryptedDataList(in); + try + { + return new PGPEncryptedDataList(in); + } + catch (IllegalArgumentException e) + { + throw new MalformedPacketException("Malformed encrypted data.", e); + } case PacketTags.ONE_PASS_SIGNATURE: l = new ArrayList(); @@ -167,6 +175,14 @@ public Object nextObject() return new PGPMarker(in); case PacketTags.PADDING: return new PGPPadding(in); + case PacketTags.MOD_DETECTION_CODE: + return new UnknownPacket(PacketTags.MOD_DETECTION_CODE, in); + case PacketTags.TRUST: + return new UnknownPacket(PacketTags.TRUST, in); + case PacketTags.USER_ID: + return new UnknownPacket(PacketTags.USER_ID, in); + case PacketTags.USER_ATTRIBUTE: + return new UnknownPacket(PacketTags.USER_ATTRIBUTE, in); } int tag = in.nextPacketTag(); diff --git a/pg/src/main/java/org/bouncycastle/openpgp/PGPSignature.java b/pg/src/main/java/org/bouncycastle/openpgp/PGPSignature.java index 7688b94600..81aa7ca16b 100644 --- a/pg/src/main/java/org/bouncycastle/openpgp/PGPSignature.java +++ b/pg/src/main/java/org/bouncycastle/openpgp/PGPSignature.java @@ -804,12 +804,20 @@ else if (getKeyAlgorithm() == PublicKeyAlgorithmTags.EDDSA_LEGACY) byte[] b = BigIntegers.asUnsignedByteArray(sigValues[1].getValue()); if (a.length + b.length > Ed25519.SIGNATURE_SIZE) { + if (a.length > Ed448.PUBLIC_KEY_SIZE || b.length > Ed448.SIGNATURE_SIZE) + { + throw new PGPException("Malformed Ed448 signature encoding (too long)."); + } signature = new byte[Ed448.SIGNATURE_SIZE]; System.arraycopy(a, 0, signature, Ed448.PUBLIC_KEY_SIZE - a.length, a.length); System.arraycopy(b, 0, signature, Ed448.SIGNATURE_SIZE - b.length, b.length); } else { + if (a.length > Ed25519.PUBLIC_KEY_SIZE || b.length > Ed25519.SIGNATURE_SIZE) + { + throw new PGPException("Malformed Ed25519 signature encoding (too long)."); + } signature = new byte[Ed25519.SIGNATURE_SIZE]; System.arraycopy(a, 0, signature, Ed25519.PUBLIC_KEY_SIZE - a.length, a.length); System.arraycopy(b, 0, signature, Ed25519.SIGNATURE_SIZE - b.length, b.length); diff --git a/pg/src/main/java/org/bouncycastle/openpgp/operator/bc/BcKeyFingerprintCalculator.java b/pg/src/main/java/org/bouncycastle/openpgp/operator/bc/BcKeyFingerprintCalculator.java index 9a4c7e2c07..07c2bd42e6 100644 --- a/pg/src/main/java/org/bouncycastle/openpgp/operator/bc/BcKeyFingerprintCalculator.java +++ b/pg/src/main/java/org/bouncycastle/openpgp/operator/bc/BcKeyFingerprintCalculator.java @@ -25,6 +25,10 @@ public byte[] calculateFingerprint(PublicKeyPacket publicPk) if (publicPk.getVersion() <= PublicKeyPacket.VERSION_3) { + if (!(key instanceof RSAPublicBCPGKey)) + { + throw new PGPException("Version 3 OpenPGP keys can only use RSA. Found " + key.getClass().getName()); + } RSAPublicBCPGKey rK = (RSAPublicBCPGKey)key; try diff --git a/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaKeyFingerprintCalculator.java b/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaKeyFingerprintCalculator.java index ba57198af1..4737059dad 100644 --- a/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaKeyFingerprintCalculator.java +++ b/pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JcaKeyFingerprintCalculator.java @@ -66,6 +66,10 @@ public byte[] calculateFingerprint(PublicKeyPacket publicPk) if (publicPk.getVersion() <= PublicKeyPacket.VERSION_3) { + if (!(key instanceof RSAPublicBCPGKey)) + { + throw new PGPException("Version 3 OpenPGP keys can only use RSA. Found " + key.getClass().getName()); + } RSAPublicBCPGKey rK = (RSAPublicBCPGKey)key; try