AzureAD / azure-activedirectory-identitymodel-extensions-for-dotnet

IdentityModel extensions for .Net
MIT License
1.06k stars 401 forks source link

Version 5.4 and 5.5 trigger “XML external entity injection via external file” during security scans by Acunetix, version 5.3 does not trigger this vulnerability. #1269

Closed tonydistler closed 5 years ago

tonydistler commented 5 years ago

Version 5.4 and 5.5 trigger “XML external entity injection via external file” during security scans by Acunetix, version 5.3 does not trigger this vulnerability. This appears to be due to 5.4 and 5.5 resolving file references in passed XML strings. 5.3 set the XmlResolver to null which stopped this behavior. In 5.4 and 5.5 the XmlResolver is not set to null. This code is located in in Microsoft.IdentityModel.Protocols.WsFederation\WsFederationMessage.cs.

5.3 line 254:

                var settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Prohibit };
#if NET45 || NET451
                settings.XmlResolver = null;
#endif
                XmlReader xmlReader = XmlReader.Create(sr, settings);

5.5 line 254 (settings variable is not used):

                var settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Prohibit };
#if NET45 || NET451
                settings.XmlResolver = null;
#endif

#if NETSTANDARD1_4
                var xmlReader = XmlDictionaryReader.CreateTextReader(Encoding.UTF8.GetBytes(Wresult), XmlDictionaryReaderQuotas.Max);
#else
                var xmlReader = new XmlTextReader(sr);
#endif
brentschmaltz commented 5 years ago

@tonydistler thanks, we will get this into our 5.6.0 release.

brentschmaltz commented 5 years ago

@tonydistler Fixed in commit: 94b00cf6bd804be3a90e3101fb751ee05935ecc3