diff --git a/docs/releasenotes.html b/docs/releasenotes.html
index 044db86a95..e1162240d1 100644
--- a/docs/releasenotes.html
+++ b/docs/releasenotes.html
@@ -23,6 +23,7 @@
2.0 Release History
Date: 2026, TBD
2.1.2 Defects Fixed
+- 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.
- The soft-fail hard limit in org.bouncycastle.pkix.jcajce.X509RevocationChecker (Builder.setSoftFailHardLimit) compared the elapsed downtime against the limit with a strict "<", 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 ("<="), so the limit fires at maxTime as documented and the maxTime = 0 case deterministically hard-fails the second failure regardless of sub-millisecond timing.
- 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).
- 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).
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 55b2291587..1ed8af8ed5 100644
--- a/pg/src/main/java/org/bouncycastle/bcpg/sig/NotationData.java
+++ b/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.");
}
diff --git a/pg/src/test/java/org/bouncycastle/bcpg/test/SignatureSubpacketsTest.java b/pg/src/test/java/org/bouncycastle/bcpg/test/SignatureSubpacketsTest.java
index 30ebfe08e5..95f8e34920 100644
--- a/pg/src/test/java/org/bouncycastle/bcpg/test/SignatureSubpacketsTest.java
+++ b/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));