fix(notifications): prohibit DTD processing in EwsResponseParser (XXE guard)
The review asked for a regression test pinning DTD-prohibited parsing. Writing it showed the premise was wrong: XDocument.Parse permits an internal DTD subset and expands its entities, so a DOCTYPE-bearing response body parsed fine and the guard did not exist (entity-expansion DoS on external input). Parse now goes through XmlReader with DtdProcessing.Prohibit and a null XmlResolver. The regression test feeds a DOCTYPE + ENTITY payload shaped as a well-formed EWS error response, so it fails if DtdProcessing is ever loosened rather than passing for the unrelated-XML reason.
This commit is contained in:
@@ -1,3 +1,4 @@
|
|||||||
|
using System.Xml;
|
||||||
using System.Xml.Linq;
|
using System.Xml.Linq;
|
||||||
|
|
||||||
namespace ZB.MOM.WW.ScadaBridge.NotificationService.Ews;
|
namespace ZB.MOM.WW.ScadaBridge.NotificationService.Ews;
|
||||||
@@ -64,9 +65,20 @@ public static class EwsResponseParser
|
|||||||
XDocument document;
|
XDocument document;
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
document = XDocument.Parse(responseBody);
|
// The body is external input, so DTDs are prohibited outright: LINQ-to-XML's own
|
||||||
|
// XDocument.Parse permits an internal DTD subset and expands its entities (verified),
|
||||||
|
// which is an entity-expansion DoS on a response body. A null resolver additionally
|
||||||
|
// blocks external entity/DTD fetches (XXE).
|
||||||
|
var settings = new XmlReaderSettings
|
||||||
|
{
|
||||||
|
DtdProcessing = DtdProcessing.Prohibit,
|
||||||
|
XmlResolver = null,
|
||||||
|
};
|
||||||
|
|
||||||
|
using var reader = XmlReader.Create(new StringReader(responseBody), settings);
|
||||||
|
document = XDocument.Load(reader);
|
||||||
}
|
}
|
||||||
catch (System.Xml.XmlException)
|
catch (XmlException)
|
||||||
{
|
{
|
||||||
return UnparseableResult;
|
return UnparseableResult;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -112,6 +112,45 @@ public class EwsResponseParserTests
|
|||||||
Assert.Equal("An internal server error occurred.", result.MessageText);
|
Assert.Equal("An internal server error occurred.", result.MessageText);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Parse_PayloadWithDtdEntityDeclaration_IsRejectedAsUnparseable()
|
||||||
|
{
|
||||||
|
// Pins DTD-prohibited parsing (XXE guard). The response body is external input, so the
|
||||||
|
// parser must never process a DOCTYPE. XDocument.Parse prohibits DTDs by default; a
|
||||||
|
// refactor that supplies XmlReaderSettings with a looser DtdProcessing fails here.
|
||||||
|
//
|
||||||
|
// The payload is deliberately a well-formed EWS response so the assertion has teeth: with
|
||||||
|
// DTDs prohibited the DOCTYPE itself throws and the body is Unparseable, whereas any
|
||||||
|
// DtdProcessing.Parse configuration expands the internal entity and yields Kind=Error.
|
||||||
|
// The external file:/// entity is declared alongside it as the XXE payload that must
|
||||||
|
// likewise never be reached.
|
||||||
|
const string xxe = """
|
||||||
|
<?xml version="1.0" encoding="utf-8"?>
|
||||||
|
<!DOCTYPE Envelope [
|
||||||
|
<!ENTITY xxe SYSTEM "file:///etc/passwd">
|
||||||
|
<!ENTITY expanded "server busy">
|
||||||
|
]>
|
||||||
|
<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/">
|
||||||
|
<s:Body>
|
||||||
|
<m:CreateItemResponse xmlns:m="http://schemas.microsoft.com/exchange/services/2006/messages">
|
||||||
|
<m:ResponseMessages>
|
||||||
|
<m:CreateItemResponseMessage ResponseClass="Error">
|
||||||
|
<m:MessageText>&expanded;</m:MessageText>
|
||||||
|
<m:ResponseCode>ErrorServerBusy</m:ResponseCode>
|
||||||
|
</m:CreateItemResponseMessage>
|
||||||
|
</m:ResponseMessages>
|
||||||
|
</m:CreateItemResponse>
|
||||||
|
</s:Body>
|
||||||
|
</s:Envelope>
|
||||||
|
""";
|
||||||
|
|
||||||
|
var result = EwsResponseParser.Parse(xxe);
|
||||||
|
|
||||||
|
Assert.Equal(EwsResponseKind.Unparseable, result.Kind);
|
||||||
|
Assert.Null(result.ResponseCode);
|
||||||
|
Assert.Null(result.MessageText);
|
||||||
|
}
|
||||||
|
|
||||||
[Theory]
|
[Theory]
|
||||||
[InlineData("not xml")]
|
[InlineData("not xml")]
|
||||||
[InlineData("")]
|
[InlineData("")]
|
||||||
|
|||||||
Reference in New Issue
Block a user