OpenPGP: Mitigate some RuntimeExceptions uncovered using Fuzzing

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

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);
     }
 

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];

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++)

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);
+    }
+}

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);
     }

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;
+    }
 }

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);
                 }
             }
         }

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);
         }
     }
 

pg/src/main/java/org/bouncycastle/bcpg/SecretKeyPacket.java

@@ -7,13 +7,17 @@
 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.
  */
 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);
         }