diff options
| -rw-r--r-- | crypto/src/asn1/Asn1InputStream.cs | 2 | ||||
| -rw-r--r-- | crypto/src/asn1/Asn1RelativeOid.cs | 84 | ||||
| -rw-r--r-- | crypto/src/asn1/DerObjectIdentifier.cs | 76 |
3 files changed, 108 insertions, 54 deletions
diff --git a/crypto/src/asn1/Asn1InputStream.cs b/crypto/src/asn1/Asn1InputStream.cs index a1bc987cc..3b5eaaa95 100644 --- a/crypto/src/asn1/Asn1InputStream.cs +++ b/crypto/src/asn1/Asn1InputStream.cs @@ -392,11 +392,13 @@ namespace Org.BouncyCastle.Asn1 } case Asn1Tags.ObjectIdentifier: { + DerObjectIdentifier.CheckContentsLength(defIn.Remaining); bool usedBuffer = GetBuffer(defIn, tmpBuffers, out var contents); return DerObjectIdentifier.CreatePrimitive(contents, clone: usedBuffer); } case Asn1Tags.RelativeOid: { + Asn1RelativeOid.CheckContentsLength(defIn.Remaining); bool usedBuffer = GetBuffer(defIn, tmpBuffers, out var contents); return Asn1RelativeOid.CreatePrimitive(contents, clone: usedBuffer); } diff --git a/crypto/src/asn1/Asn1RelativeOid.cs b/crypto/src/asn1/Asn1RelativeOid.cs index 3e2b54c9e..f1b9bc274 100644 --- a/crypto/src/asn1/Asn1RelativeOid.cs +++ b/crypto/src/asn1/Asn1RelativeOid.cs @@ -23,6 +23,16 @@ namespace Org.BouncyCastle.Asn1 } } + /// <summary>Implementation limit on the length of the contents octets for a Relative OID.</summary> + /// <remarks> + /// We adopt the same value used by OpenJDK for Object Identifier. In theory there is no limit on the length of + /// the contents, or the number of subidentifiers, or the length of individual subidentifiers. In practice, + /// supporting arbitrary lengths can lead to issues, e.g. denial-of-service attacks when attempting to convert a + /// parsed value to its (decimal) string form. + /// </remarks> + private const int MaxContentsLength = 4096; + private const int MaxIdentifierLength = MaxContentsLength * 4 - 1; + public static Asn1RelativeOid FromContents(byte[] contents) { if (contents == null) @@ -69,14 +79,18 @@ namespace Org.BouncyCastle.Asn1 { if (identifier == null) throw new ArgumentNullException(nameof(identifier)); - if (!IsValidIdentifier(identifier, 0)) + if (identifier.Length <= MaxIdentifierLength && IsValidIdentifier(identifier, from: 0)) { - oid = default; - return false; + byte[] contents = ParseIdentifier(identifier); + if (contents.Length <= MaxContentsLength) + { + oid = new Asn1RelativeOid(contents, identifier); + return true; + } } - oid = new Asn1RelativeOid(ParseIdentifier(identifier), identifier); - return true; + oid = default; + return false; } private const long LongLimit = (long.MaxValue >> 7) - 0x7F; @@ -88,31 +102,13 @@ namespace Org.BouncyCastle.Asn1 public Asn1RelativeOid(string identifier) { - if (identifier == null) - throw new ArgumentNullException("identifier"); - if (!IsValidIdentifier(identifier, 0)) - throw new FormatException("string " + identifier + " not a relative OID"); + CheckIdentifier(identifier); - m_contents = ParseIdentifier(identifier); - m_identifier = identifier; - } - - private Asn1RelativeOid(Asn1RelativeOid oid, string branchID) - { - if (!IsValidIdentifier(branchID, 0)) - throw new FormatException("string " + branchID + " not a valid relative OID branch"); - - m_contents = Arrays.Concatenate(oid.m_contents, ParseIdentifier(branchID)); - m_identifier = oid.GetID() + "." + branchID; - } - - private Asn1RelativeOid(byte[] contents, bool clone) - { - if (!IsValidContents(contents)) - throw new ArgumentException("invalid relative OID contents", nameof(contents)); + byte[] contents = ParseIdentifier(identifier); + CheckContentsLength(contents.Length); - m_contents = clone ? Arrays.Clone(contents) : contents; - m_identifier = null; + m_contents = contents; + m_identifier = identifier; } private Asn1RelativeOid(byte[] contents, string identifier) @@ -123,7 +119,14 @@ namespace Org.BouncyCastle.Asn1 public virtual Asn1RelativeOid Branch(string branchID) { - return new Asn1RelativeOid(this, branchID); + CheckIdentifier(branchID); + + byte[] branchContents = ParseIdentifier(branchID); + CheckContentsLength(m_contents.Length + branchContents.Length); + + return new Asn1RelativeOid( + contents: Arrays.Concatenate(m_contents, branchContents), + identifier: GetID() + "." + branchID); } public string GetID() @@ -168,8 +171,26 @@ namespace Org.BouncyCastle.Asn1 return new PrimitiveDerEncoding(tagClass, tagNo, m_contents); } + internal static void CheckContentsLength(int contentsLength) + { + if (contentsLength > MaxContentsLength) + throw new ArgumentException("exceeded relative OID contents length limit"); + } + + internal static void CheckIdentifier(string identifier) + { + if (identifier == null) + throw new ArgumentNullException("identifier"); + if (identifier.Length > MaxIdentifierLength) + throw new ArgumentException("exceeded relative OID contents length limit"); + if (!IsValidIdentifier(identifier, from: 0)) + throw new FormatException("string " + identifier + " not a valid relative OID"); + } + internal static Asn1RelativeOid CreatePrimitive(byte[] contents, bool clone) { + CheckContentsLength(contents.Length); + uint index = (uint)Arrays.GetHashCode(contents); index ^= index >> 24; @@ -181,7 +202,10 @@ namespace Org.BouncyCastle.Asn1 if (originalEntry != null && Arrays.AreEqual(contents, originalEntry.m_contents)) return originalEntry; - var newEntry = new Asn1RelativeOid(contents, clone); + if (!IsValidContents(contents)) + throw new ArgumentException("invalid relative OID contents", nameof(contents)); + + var newEntry = new Asn1RelativeOid(clone ? Arrays.Clone(contents) : contents, identifier: null); var exchangedEntry = Interlocked.CompareExchange(ref Cache[index], newEntry, originalEntry); if (exchangedEntry != originalEntry) diff --git a/crypto/src/asn1/DerObjectIdentifier.cs b/crypto/src/asn1/DerObjectIdentifier.cs index 51acd232c..4a68d541c 100644 --- a/crypto/src/asn1/DerObjectIdentifier.cs +++ b/crypto/src/asn1/DerObjectIdentifier.cs @@ -23,6 +23,16 @@ namespace Org.BouncyCastle.Asn1 } } + /// <summary>Implementation limit on the length of the contents octets for an Object Identifier.</summary> + /// <remarks> + /// We adopt the same value used by OpenJDK. In theory there is no limit on the length of the contents, or the + /// number of subidentifiers, or the length of individual subidentifiers. In practice, supporting arbitrary + /// lengths can lead to issues, e.g. denial-of-service attacks when attempting to convert a parsed value to its + /// (decimal) string form. + /// </remarks> + private const int MaxContentsLength = 4096; + private const int MaxIdentifierLength = MaxContentsLength * 4 + 1; + public static DerObjectIdentifier FromContents(byte[] contents) { if (contents == null) @@ -86,14 +96,18 @@ namespace Org.BouncyCastle.Asn1 { if (identifier == null) throw new ArgumentNullException(nameof(identifier)); - if (!IsValidIdentifier(identifier)) + if (identifier.Length <= MaxIdentifierLength && IsValidIdentifier(identifier)) { - oid = default; - return false; + byte[] contents = ParseIdentifier(identifier); + if (contents.Length <= MaxContentsLength) + { + oid = new DerObjectIdentifier(contents, identifier); + return true; + } } - oid = new DerObjectIdentifier(ParseIdentifier(identifier), identifier); - return true; + oid = default; + return false; } private const long LongLimit = (long.MaxValue >> 7) - 0x7F; @@ -105,22 +119,13 @@ namespace Org.BouncyCastle.Asn1 public DerObjectIdentifier(string identifier) { - if (identifier == null) - throw new ArgumentNullException("identifier"); - if (!IsValidIdentifier(identifier)) - throw new FormatException("string " + identifier + " not an OID"); - - m_contents = ParseIdentifier(identifier); - m_identifier = identifier; - } + CheckIdentifier(identifier); - private DerObjectIdentifier(byte[] contents, bool clone) - { - if (!Asn1RelativeOid.IsValidContents(contents)) - throw new ArgumentException("invalid OID contents", nameof(contents)); + byte[] contents = ParseIdentifier(identifier); + CheckContentsLength(contents.Length); - m_contents = clone ? Arrays.Clone(contents) : contents; - m_identifier = null; + m_contents = contents; + m_identifier = identifier; } private DerObjectIdentifier(byte[] contents, string identifier) @@ -131,11 +136,13 @@ namespace Org.BouncyCastle.Asn1 public virtual DerObjectIdentifier Branch(string branchID) { - if (!Asn1RelativeOid.IsValidIdentifier(branchID, 0)) - throw new FormatException("string " + branchID + " not a valid OID branch"); + Asn1RelativeOid.CheckIdentifier(branchID); + + byte[] branchContents = Asn1RelativeOid.ParseIdentifier(branchID); + CheckContentsLength(m_contents.Length + branchContents.Length); return new DerObjectIdentifier( - contents: Arrays.Concatenate(m_contents, Asn1RelativeOid.ParseIdentifier(branchID)), + contents: Arrays.Concatenate(m_contents, branchContents), identifier: GetID() + "." + branchID); } @@ -195,8 +202,26 @@ namespace Org.BouncyCastle.Asn1 return new PrimitiveDerEncoding(tagClass, tagNo, m_contents); } + internal static void CheckContentsLength(int contentsLength) + { + if (contentsLength > MaxContentsLength) + throw new ArgumentException("exceeded OID contents length limit"); + } + + internal static void CheckIdentifier(string identifier) + { + if (identifier == null) + throw new ArgumentNullException("identifier"); + if (identifier.Length > MaxIdentifierLength) + throw new ArgumentException("exceeded OID contents length limit"); + if (!IsValidIdentifier(identifier)) + throw new FormatException("string " + identifier + " not a valid OID"); + } + internal static DerObjectIdentifier CreatePrimitive(byte[] contents, bool clone) { + CheckContentsLength(contents.Length); + uint index = (uint)Arrays.GetHashCode(contents); index ^= index >> 20; @@ -207,7 +232,10 @@ namespace Org.BouncyCastle.Asn1 if (originalEntry != null && Arrays.AreEqual(contents, originalEntry.m_contents)) return originalEntry; - var newEntry = new DerObjectIdentifier(contents, clone); + if (!Asn1RelativeOid.IsValidContents(contents)) + throw new ArgumentException("invalid OID contents", nameof(contents)); + + var newEntry = new DerObjectIdentifier(clone ? Arrays.Clone(contents) : contents, identifier: null); var exchangedEntry = Interlocked.CompareExchange(ref Cache[index], newEntry, originalEntry); if (exchangedEntry != originalEntry) @@ -228,7 +256,7 @@ namespace Org.BouncyCastle.Asn1 if (first < '0' || first > '2') return false; - if (!Asn1RelativeOid.IsValidIdentifier(identifier, 2)) + if (!Asn1RelativeOid.IsValidIdentifier(identifier, from: 2)) return false; if (first == '2') |