Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 8f968b9

Browse files
author
Ian Hays
committed
Merged PR 117476: [2.1-MSRC] Misc SignedXML fixes
1 parent 35e4886 commit 8f968b9

29 files changed

+846
-72
lines changed

‎src/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CipherReference.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ public override void LoadXml(XmlElement value)
7474
throw new ArgumentNullException(nameof(value));
7575

7676
ReferenceType = value.LocalName;
77-
Uri = Utils.GetAttribute(value, "URI", EncryptedXml.XmlEncNamespaceUrl);
77+
string uri = Utils.GetAttribute(value, "URI", EncryptedXml.XmlEncNamespaceUrl);
78+
Uri = uri ?? throw new CryptographicException(SR.Cryptography_Xml_UriRequired);
7879

7980
// Transforms
8081
XmlNamespaceManager nsm = new XmlNamespaceManager(value.OwnerDocument.NameTable);

‎src/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/CryptoHelpers.cs

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,21 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
using System.Diagnostics.CodeAnalysis;
5+
using System;
6+
using System.Collections.Generic;
7+
using System.Linq;
8+
using System.Text;
9+
using System.Threading.Tasks;
610

711
namespace System.Security.Cryptography.Xml
812
{
913
internal static class CryptoHelpers
1014
{
11-
[SuppressMessage("Microsoft.Security", "CA5350", Justification = "SHA1 needed for compat.")]
12-
[SuppressMessage("Microsoft.Security", "CA5351", Justification = "HMACMD5 needed for compat.")]
13-
public static object CreateFromName(string name)
15+
private static readonly char[] _invalidChars = new char[] { ',', '`', '[', '*', '&' };
16+
17+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Security", "CA5350", Justification = "SHA1 needed for compat.")]
18+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Security", "CA5351", Justification = "HMACMD5 needed for compat.")]
19+
public static object CreateFromKnownName(string name)
1420
{
1521
switch (name)
1622
{
@@ -73,7 +79,23 @@ public static object CreateFromName(string name)
7379
return TripleDES.Create();
7480
}
7581

76-
return CryptoConfig.CreateFromName(name);
82+
return null;
83+
}
84+
85+
public static T CreateFromName<T>(string name) where T : class
86+
{
87+
if (name == null || name.IndexOfAny(_invalidChars) >= 0)
88+
{
89+
return null;
90+
}
91+
try
92+
{
93+
return (CreateFromKnownName(name) ?? CryptoConfig.CreateFromName(name)) as T;
94+
}
95+
catch (Exception)
96+
{
97+
return null;
98+
}
7799
}
78100
}
79101
}

‎src/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/DSASignatureDescription.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,15 @@ public DSASignatureDescription()
2020

2121
public sealed override AsymmetricSignatureDeformatter CreateDeformatter(AsymmetricAlgorithm key)
2222
{
23-
var item = (AsymmetricSignatureDeformatter)CryptoHelpers.CreateFromName(DeformatterAlgorithm);
23+
var item = (AsymmetricSignatureDeformatter)CryptoConfig.CreateFromName(DeformatterAlgorithm);
2424
item.SetKey(key);
2525
item.SetHashAlgorithm(HashAlgorithm);
2626
return item;
2727
}
2828

2929
public sealed override AsymmetricSignatureFormatter CreateFormatter(AsymmetricAlgorithm key)
3030
{
31-
var item = (AsymmetricSignatureFormatter)CryptoHelpers.CreateFromName(FormatterAlgorithm);
31+
var item = (AsymmetricSignatureFormatter)CryptoConfig.CreateFromName(FormatterAlgorithm);
3232
item.SetKey(key);
3333
item.SetHashAlgorithm(HashAlgorithm);
3434
return item;

‎src/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedReference.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,11 @@ public virtual void LoadXml(XmlElement value)
112112
throw new ArgumentNullException(nameof(value));
113113

114114
ReferenceType = value.LocalName;
115-
Uri = Utils.GetAttribute(value, "URI", EncryptedXml.XmlEncNamespaceUrl);
115+
116+
string uri = Utils.GetAttribute(value, "URI", EncryptedXml.XmlEncNamespaceUrl);
117+
if (uri == null)
118+
throw new ArgumentNullException(SR.Cryptography_Xml_UriRequired);
119+
Uri = uri;
116120

117121
// Transforms
118122
XmlNamespaceManager nsm = new XmlNamespaceManager(value.OwnerDocument.NameTable);

‎src/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedXml.cs

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,21 +197,38 @@ private byte[] GetCipherValue(CipherData cipherData)
197197
if (cipherData.CipherReference.CipherValue != null)
198198
return cipherData.CipherReference.CipherValue;
199199
Stream decInputStream = null;
200+
if (cipherData.CipherReference.Uri == null)
201+
{
202+
throw new CryptographicException(SR.Cryptography_Xml_UriNotSupported);
203+
}
200204
// See if the CipherReference is a local URI
201205
if (cipherData.CipherReference.Uri.Length == 0)
202206
{
203207
// self referenced Uri
204208
string baseUri = (_document == null ? null : _document.BaseURI);
205209
TransformChain tc = cipherData.CipherReference.TransformChain;
210+
if (tc == null)
211+
{
212+
throw new CryptographicException(SR.Cryptography_Xml_UriNotSupported);
213+
}
206214
decInputStream = tc.TransformToOctetStream(_document, _xmlResolver, baseUri);
207215
}
208216
else if (cipherData.CipherReference.Uri[0] == '#')
209217
{
210218
string idref = Utils.ExtractIdFromLocalUri(cipherData.CipherReference.Uri);
211219
// Serialize
212-
inputStream = new MemoryStream(_encoding.GetBytes(GetIdElement(_document, idref).OuterXml));
220+
XmlElement idElem = GetIdElement(_document, idref);
221+
if (idElem == null || idElem.OuterXml == null)
222+
{
223+
throw new CryptographicException(SR.Cryptography_Xml_UriNotSupported);
224+
}
225+
inputStream = new MemoryStream(_encoding.GetBytes(idElem.OuterXml));
213226
string baseUri = (_document == null ? null : _document.BaseURI);
214227
TransformChain tc = cipherData.CipherReference.TransformChain;
228+
if (tc == null)
229+
{
230+
throw new CryptographicException(SR.Cryptography_Xml_UriNotSupported);
231+
}
215232
decInputStream = tc.TransformToOctetStream(inputStream, _xmlResolver, baseUri);
216233
}
217234
else
@@ -361,7 +378,11 @@ public virtual SymmetricAlgorithm GetDecryptionKey(EncryptedData encryptedData,
361378
if (key == null)
362379
throw new CryptographicException(SR.Cryptography_Xml_MissingDecryptionKey);
363380

364-
SymmetricAlgorithm symAlg = (SymmetricAlgorithm)CryptoHelpers.CreateFromName(symmetricAlgorithmUri);
381+
SymmetricAlgorithm symAlg = CryptoHelpers.CreateFromName<SymmetricAlgorithm>(symmetricAlgorithmUri);
382+
if (symAlg == null)
383+
{
384+
throw new CryptographicException(SR.Cryptography_Xml_MissingAlgorithm);
385+
}
365386
symAlg.Key = key;
366387
return symAlg;
367388
}
@@ -394,6 +415,10 @@ public virtual byte[] DecryptEncryptedKey(EncryptedKey encryptedKey)
394415
object kek = _keyNameMapping[keyName];
395416
if (kek != null)
396417
{
418+
if (encryptedKey.CipherData == null || encryptedKey.CipherData.CipherValue == null)
419+
{
420+
throw new CryptographicException(SR.Cryptography_Xml_MissingAlgorithm);
421+
}
397422
// kek is either a SymmetricAlgorithm or an RSA key, otherwise, we wouldn't be able to insert it in the hash table
398423
if (kek is SymmetricAlgorithm)
399424
return EncryptedXml.DecryptKey(encryptedKey.CipherData.CipherValue, (SymmetricAlgorithm)kek);
@@ -414,6 +439,10 @@ public virtual byte[] DecryptEncryptedKey(EncryptedKey encryptedKey)
414439
{
415440
if (privateKey != null)
416441
{
442+
if (encryptedKey.CipherData == null || encryptedKey.CipherData.CipherValue == null)
443+
{
444+
throw new CryptographicException(SR.Cryptography_Xml_MissingAlgorithm);
445+
}
417446
fOAEP = (encryptedKey.EncryptionMethod != null && encryptedKey.EncryptionMethod.KeyAlgorithm == EncryptedXml.XmlEncRSAOAEPUrl);
418447
return EncryptedXml.DecryptKey(encryptedKey.CipherData.CipherValue, privateKey, fOAEP);
419448
}
@@ -456,7 +485,16 @@ public virtual byte[] DecryptEncryptedKey(EncryptedKey encryptedKey)
456485
if (encryptionKey != null)
457486
{
458487
// this is a symmetric algorithm for sure
459-
SymmetricAlgorithm symAlg = (SymmetricAlgorithm)CryptoHelpers.CreateFromName(encryptedKey.EncryptionMethod.KeyAlgorithm);
488+
SymmetricAlgorithm symAlg = CryptoHelpers.CreateFromName<SymmetricAlgorithm>(encryptedKey.EncryptionMethod.KeyAlgorithm);
489+
if (symAlg == null)
490+
{
491+
throw new CryptographicException(SR.Cryptography_Xml_MissingAlgorithm);
492+
}
493+
symAlg.Key = encryptionKey;
494+
if (encryptedKey.CipherData == null || encryptedKey.CipherData.CipherValue == null)
495+
{
496+
throw new CryptographicException(SR.Cryptography_Xml_MissingAlgorithm);
497+
}
460498
symAlg.Key = encryptionKey;
461499
return EncryptedXml.DecryptKey(encryptedKey.CipherData.CipherValue, symAlg);
462500
}

‎src/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/KeyInfo.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ public void LoadXml(XmlElement value)
7171

7272
XmlElement keyInfoElement = value;
7373
_id = Utils.GetAttribute(keyInfoElement, "Id", SignedXml.XmlDsigNamespaceUrl);
74+
if (!Utils.VerifyAttributes(keyInfoElement, "Id"))
75+
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "KeyInfo");
7476

7577
XmlNode child = keyInfoElement.FirstChild;
7678
while (child != null)
@@ -83,6 +85,10 @@ public void LoadXml(XmlElement value)
8385
// Special-case handling for KeyValue -- we have to go one level deeper
8486
if (kicString == "http://www.w3.org/2000/09/xmldsig# KeyValue")
8587
{
88+
if (!Utils.VerifyAttributes(elem, (string[])null))
89+
{
90+
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "KeyInfo/KeyValue");
91+
}
8692
XmlNodeList nodeList2 = elem.ChildNodes;
8793
foreach (XmlNode node2 in nodeList2)
8894
{
@@ -94,7 +100,8 @@ public void LoadXml(XmlElement value)
94100
}
95101
}
96102
}
97-
KeyInfoClause keyInfoClause = (KeyInfoClause)CryptoHelpers.CreateFromName(kicString);
103+
104+
KeyInfoClause keyInfoClause = CryptoHelpers.CreateFromName<KeyInfoClause>(kicString);
98105
// if we don't know what kind of KeyInfoClause we're looking at, use a generic KeyInfoNode:
99106
if (keyInfoClause == null)
100107
keyInfoClause = new KeyInfoNode();

‎src/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/RSAPKCS1SignatureDescription.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@ public RSAPKCS1SignatureDescription(string hashAlgorithmName)
1616

1717
public sealed override AsymmetricSignatureDeformatter CreateDeformatter(AsymmetricAlgorithm key)
1818
{
19-
var item = (AsymmetricSignatureDeformatter)CryptoHelpers.CreateFromName(DeformatterAlgorithm);
19+
var item = (AsymmetricSignatureDeformatter)CryptoConfig.CreateFromName(DeformatterAlgorithm);
2020
item.SetKey(key);
2121
item.SetHashAlgorithm(DigestAlgorithm);
2222
return item;
2323
}
2424

2525
public sealed override AsymmetricSignatureFormatter CreateFormatter(AsymmetricAlgorithm key)
2626
{
27-
var item = (AsymmetricSignatureFormatter)CryptoHelpers.CreateFromName(FormatterAlgorithm);
27+
var item = (AsymmetricSignatureFormatter)CryptoConfig.CreateFromName(FormatterAlgorithm);
2828
item.SetKey(key);
2929
item.SetHashAlgorithm(DigestAlgorithm);
3030
return item;

‎src/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/Reference.cs

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -219,25 +219,52 @@ public void LoadXml(XmlElement value)
219219
_id = Utils.GetAttribute(value, "Id", SignedXml.XmlDsigNamespaceUrl);
220220
_uri = Utils.GetAttribute(value, "URI", SignedXml.XmlDsigNamespaceUrl);
221221
_type = Utils.GetAttribute(value, "Type", SignedXml.XmlDsigNamespaceUrl);
222+
if (!Utils.VerifyAttributes(value, new string[] { "Id", "URI", "Type" }))
223+
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Reference");
222224

223225
XmlNamespaceManager nsm = new XmlNamespaceManager(value.OwnerDocument.NameTable);
224226
nsm.AddNamespace("ds", SignedXml.XmlDsigNamespaceUrl);
225227

226228
// Transforms
229+
bool hasTransforms = false;
227230
TransformChain = new TransformChain();
228-
XmlElement transformsElement = value.SelectSingleNode("ds:Transforms", nsm) as XmlElement;
229-
if (transformsElement != null)
231+
XmlNodeList transformsNodes = value.SelectNodes("ds:Transforms", nsm);
232+
if (transformsNodes != null && transformsNodes.Count != 0)
230233
{
234+
if (transformsNodes.Count > 1)
235+
{
236+
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Reference/Transforms");
237+
}
238+
hasTransforms = true;
239+
XmlElement transformsElement = transformsNodes[0] as XmlElement;
240+
if (!Utils.VerifyAttributes(transformsElement, (string[])null))
241+
{
242+
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Reference/Transforms");
243+
}
231244
XmlNodeList transformNodes = transformsElement.SelectNodes("ds:Transform", nsm);
232245
if (transformNodes != null)
233246
{
247+
if (transformNodes.Count != transformsElement.SelectNodes("*").Count)
248+
{
249+
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Reference/Transforms");
250+
}
251+
if (transformNodes.Count > Utils.MaxTransformsPerReference)
252+
{
253+
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Reference/Transforms");
254+
}
234255
foreach (XmlNode transformNode in transformNodes)
235256
{
236257
XmlElement transformElement = transformNode as XmlElement;
237258
string algorithm = Utils.GetAttribute(transformElement, "Algorithm", SignedXml.XmlDsigNamespaceUrl);
238-
Transform transform = CryptoHelpers.CreateFromName(algorithm) as Transform;
259+
if (algorithm == null || !Utils.VerifyAttributes(transformElement, "Algorithm"))
260+
{
261+
throw new CryptographicException(SR.Cryptography_Xml_UnknownTransform);
262+
}
263+
Transform transform = CryptoHelpers.CreateFromName<Transform>(algorithm);
239264
if (transform == null)
265+
{
240266
throw new CryptographicException(SR.Cryptography_Xml_UnknownTransform);
267+
}
241268
AddTransform(transform);
242269
// let the transform read the children of the transformElement for data
243270
transform.LoadInnerXml(transformElement.ChildNodes);
@@ -267,16 +294,27 @@ public void LoadXml(XmlElement value)
267294
}
268295

269296
// DigestMethod
270-
XmlElement digestMethodElement = value.SelectSingleNode("ds:DigestMethod", nsm) as XmlElement;
271-
if (digestMethodElement == null)
297+
XmlNodeList digestMethodNodes = value.SelectNodes("ds:DigestMethod", nsm);
298+
if (digestMethodNodes == null || digestMethodNodes.Count == 0 || digestMethodNodes.Count > 1)
272299
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Reference/DigestMethod");
300+
XmlElement digestMethodElement = digestMethodNodes[0] as XmlElement;
273301
_digestMethod = Utils.GetAttribute(digestMethodElement, "Algorithm", SignedXml.XmlDsigNamespaceUrl);
302+
if (_digestMethod == null || !Utils.VerifyAttributes(digestMethodElement, "Algorithm"))
303+
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Reference/DigestMethod");
304+
274305

275306
// DigestValue
276-
XmlElement digestValueElement = value.SelectSingleNode("ds:DigestValue", nsm) as XmlElement;
277-
if (digestValueElement == null)
307+
XmlNodeList digestValueNodes = value.SelectNodes("ds:DigestValue", nsm);
308+
if (digestValueNodes == null || digestValueNodes.Count == 0 || digestValueNodes.Count > 1)
278309
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Reference/DigestValue");
310+
XmlElement digestValueElement = digestValueNodes[0] as XmlElement;
279311
_digestValue = Convert.FromBase64String(Utils.DiscardWhiteSpaces(digestValueElement.InnerText));
312+
if (!Utils.VerifyAttributes(digestValueElement, (string[])null))
313+
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Reference/DigestValue");
314+
// Verify that there aren't any extra nodes that aren't allowed
315+
int expectedChildNodeCount = hasTransforms ? 3 : 2;
316+
if (value.SelectNodes("*").Count != expectedChildNodeCount)
317+
throw new CryptographicException(SR.Cryptography_Xml_InvalidElement, "Reference");
280318

281319
// cache the Xml
282320
_cachedXml = value;
@@ -304,7 +342,7 @@ internal byte[] CalculateHashValue(XmlDocument document, CanonicalXmlNodeList re
304342
{
305343
// refList is a list of elements that might be targets of references
306344
// Now's the time to create our hashing algorithm
307-
_hashAlgorithm = CryptoHelpers.CreateFromName(_digestMethod) as HashAlgorithm;
345+
_hashAlgorithm = CryptoHelpers.CreateFromName<HashAlgorithm>(_digestMethod);
308346
if (_hashAlgorithm == null)
309347
throw new CryptographicException(SR.Cryptography_Xml_CreateHashAlgorithmFailed);
310348

0 commit comments

Comments
 (0)