fix off-by-4 header length guard in NotationData.verifyData

docs/releasenotes.html

@@ -23,6 +23,7 @@ <h2>2.0 Release History</h2>
 Date:&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; 2026, TBD
 <h3>2.1.2 Defects Fixed</h3>
 <ul>
+<li>The OpenPGP Notation Data signature subpacket parser org.bouncycastle.bcpg.sig.NotationData validated its body with a length guard that under-counted its own fixed header by four octets: the header is flags[4] || nameLength[2] || valueLength[2] (8 octets), but verifyData() compared nameLength + valueLength + 4 against data.length instead of + 8. A crafted (critical) Notation Data subpacket whose header declared more name/value data than the body carried therefore passed validation and decoded cleanly, then threw an unchecked ArrayIndexOutOfBoundsException from getNotationName() / getNotationValueBytes() when the name/value slice was copied - reachable while verifying an attacker-supplied signature, where critical notations are read. The guard now accounts for the full 8-octet header, so a short body is rejected at parse time with IllegalArgumentException (surfacing as MalformedPacketException from SignatureSubpacketInputStream), matching the truncation guards already present on the sibling Features / TrustSignature / SignatureTarget / RevocationKey / RevocationReason subpackets. Well-formed notation data is unaffected.</li>
 <li>The soft-fail hard limit in org.bouncycastle.pkix.jcajce.X509RevocationChecker (Builder.setSoftFailHardLimit) compared the elapsed downtime against the limit with a strict "&lt;", so a failure occurring exactly at maxTime was still treated as soft rather than hard - contrary to the javadoc ("At maxTime any failures will be treated as hard"). With maxTime = 0 this meant the second failure only hard-failed once a full millisecond had elapsed since the first, so on a fast machine two revocation checks within the same millisecond both soft-passed. The comparison is now inclusive ("&lt;="), so the limit fires at maxTime as documented and the maxTime = 0 case deterministically hard-fails the second failure regardless of sub-millisecond timing.</li>
 <li>KMIPInputStream (org.bouncycastle.kmip.wire) built its XMLEventReader from a bare XMLInputFactory, so a KMIP XML message carrying a DOCTYPE was processed and external SYSTEM entities were resolved during parsing - an XML External Entity (XXE) exposure permitting local file disclosure via file:// URIs, outbound requests (SSRF) via http:// URIs, and information disclosure through parse error messages. The factory is now configured with SUPPORT_DTD = false (which rejects any DOCTYPE outright) and IS_SUPPORTING_EXTERNAL_ENTITIES = false before the reader is created. KMIP messages without a DOCTYPE parse exactly as before (github #2315).</li>
 <li>Reading a GnuPG keybox (org.bouncycastle.gpg.keybox.KeyBox / BcKeyBox / JcaKeyBox) stopped at the first EMPTY_BLOB, treating that free/deleted slot as end-of-file: Blob.getInstance() fell through to returning null (the caller's end-of-file signal) for a BlobType.EMPTY_BLOB, so any key blobs following an empty blob were silently dropped - a keybox laid out as OpenPGP, OpenPGP, OpenPGP, EMPTY_BLOB, OpenPGP returned only three key blobs where gnupg reads four. An empty blob is now skipped by advancing past its declared length and parsing continues with the following blobs; a malformed empty-blob length that would fail to advance or point beyond the buffer still terminates the read (issue #2343).</li>

pg/src/main/java/org/bouncycastle/bcpg/sig/NotationData.java

@@ -47,7 +47,7 @@ private static byte[] verifyData(byte[] data)
         }
         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)
+        if (nameLength + valueLength + HEADER_FLAG_LENGTH + HEADER_NAME_LENGTH + HEADER_VALUE_LENGTH > data.length)
         {
             throw new IllegalArgumentException("Malformed notation data encoding.");
         }

pg/src/test/java/org/bouncycastle/bcpg/test/SignatureSubpacketsTest.java

@@ -7,6 +7,7 @@
 import org.bouncycastle.bcpg.SignatureSubpacketTags;
 import org.bouncycastle.bcpg.sig.Features;
 import org.bouncycastle.bcpg.sig.LibrePGPPreferredEncryptionModes;
+import org.bouncycastle.bcpg.sig.NotationData;
 import org.bouncycastle.bcpg.sig.RevocationKey;
 import org.bouncycastle.bcpg.sig.RevocationReason;
 import org.bouncycastle.bcpg.sig.SignatureTarget;
@@ -58,10 +59,11 @@ private void testLibrePGPPreferredEncryptionModesSubpacket()
     }
 
     /**
-     * The Features, TrustSignature, SignatureTarget, RevocationKey and RevocationReason
-     * subpackets index a fixed offset of their body from an accessor (e.g.
-     * {@link Features#getFeatures()} reads {@code data[0]}). A truncated body (empty, or a
-     * single octet for the two-octet subpackets) must therefore be rejected when the subpacket
+     * The Features, TrustSignature, SignatureTarget, RevocationKey, RevocationReason and
+     * NotationData subpackets index their body from an accessor (e.g.
+     * {@link Features#getFeatures()} reads {@code data[0]}, {@link NotationData#getNotationName()}
+     * copies {@code nameLength} octets from offset 8, where {@code nameLength} comes from the
+     * subpacket's own header). A truncated body must therefore be rejected when the subpacket
      * is parsed, with an {@link IllegalArgumentException}, rather than decoding cleanly and
      * throwing an {@link ArrayIndexOutOfBoundsException} later when an accessor is read. This
      * matches the existing IssuerFingerprint / IntendedRecipientFingerprint guards.
@@ -87,6 +89,15 @@ private void testTruncatedSubpacketsRejected()
         // getRevocationReason() reads data[0]
         isConstructionRejected("RevocationReason", new byte[0]);
 
+        // NotationData: header is flags[4] || nameLength[2] || valueLength[2] (8 octets);
+        // getNotationName() copies nameLength octets from offset 8 and getNotationValueBytes()
+        // copies valueLength octets from offset 8 + nameLength, so a body whose header declares
+        // more name/value data than it carries must be rejected, not decoded and later AIOOBE'd.
+        isConstructionRejected("NotationData", new byte[0]);                    // shorter than the 8-octet header
+        isConstructionRejected("NotationData", notationBody(4, 0, new byte[0])); // declares 4 name octets, none supplied
+        isConstructionRejected("NotationData", notationBody(0, 4, new byte[0])); // declares 4 value octets, none supplied
+        isConstructionRejected("NotationData", notationBody(2, 2, new byte[]{'a', 'b', 'c'})); // one value octet short
+
         // a body exactly at the minimum length must still be accepted, with working accessors
         testMinimalBodiesAccepted();
 
@@ -95,6 +106,7 @@ private void testTruncatedSubpacketsRejected()
         // rejects with a MalformedPacketException wrapping the constructor's exception.
         isWireDecodeRejected(SignatureSubpacketTags.FEATURES, 1);
         isWireDecodeRejected(SignatureSubpacketTags.TRUST_SIG, 2);
+        isWireDecodeRejected(SignatureSubpacketTags.NOTATION_DATA, notationBody(4, 0, new byte[0]));
     }
 
     private void isConstructionRejected(String name, byte[] body)
@@ -132,9 +144,25 @@ private SignatureSubpacket construct(String name, byte[] body)
         {
             return new RevocationReason(false, false, body);
         }
+        if (name.equals("NotationData"))
+        {
+            return new NotationData(false, false, body);
+        }
         throw new IllegalStateException("unknown subpacket: " + name);
     }
 
+    private static byte[] notationBody(int nameLength, int valueLength, byte[] payload)
+    {
+        // flags[4] || nameLength[2] || valueLength[2] || payload
+        byte[] body = new byte[8 + payload.length];
+        body[4] = (byte)(nameLength >>> 8);
+        body[5] = (byte)nameLength;
+        body[6] = (byte)(valueLength >>> 8);
+        body[7] = (byte)valueLength;
+        System.arraycopy(payload, 0, body, 8, payload.length);
+        return body;
+    }
+
     private void testMinimalBodiesAccepted()
     {
         Features features = new Features(false, false, new byte[]{Features.FEATURE_SEIPD_V2});
@@ -158,17 +186,29 @@ private void testMinimalBodiesAccepted()
         RevocationReason revocationReason = new RevocationReason(false, false, new byte[]{3});
         isTrue("RevocationReason code mismatch", revocationReason.getRevocationReason() == 3);
         isTrue("RevocationReason description should be empty", revocationReason.getRevocationDescription().equals(""));
+
+        NotationData notationData = new NotationData(false, false, notationBody(2, 2, new byte[]{'a', 'b', 'c', 'd'}));
+        isTrue("NotationData name mismatch", notationData.getNotationName().equals("ab"));
+        isTrue("NotationData value mismatch", notationData.getNotationValue().equals("cd"));
     }
 
     private void isWireDecodeRejected(int type, int subpacketLength)
             throws IOException
+    {
+        // a length-field-only body, all octets zero, too short for the subpacket's accessors
+        isWireDecodeRejected(type, new byte[subpacketLength - 1]);
+    }
+
+    private void isWireDecodeRejected(int type, byte[] body)
+            throws IOException
     {
         // OpenPGP signature subpacket framing: a one-octet length field (< 192) covering the
-        // type octet plus body, the type octet, then (subpacketLength - 1) body octets (left
-        // zero here so the body is too short for the subpacket's accessors).
-        byte[] encoded = new byte[1 + subpacketLength];
-        encoded[0] = (byte)subpacketLength;
+        // type octet plus body, the type octet, then the body octets (too short for the
+        // subpacket's accessors).
+        byte[] encoded = new byte[2 + body.length];
+        encoded[0] = (byte)(1 + body.length);
         encoded[1] = (byte)type;
+        System.arraycopy(body, 0, encoded, 2, body.length);
 
         SignatureSubpacketInputStream sIn = new SignatureSubpacketInputStream(
                 new ByteArrayInputStream(encoded));