microsoft / msix-packaging

MSIX SDK
MIT License
978 stars 165 forks source link

[BUG] entity reference nodes should be created when parsing XML #481

Closed eric-therond-sonarsource closed 3 years ago

eric-therond-sonarsource commented 3 years ago

MSIX SDK implements an incorrect solution to prevent XXE vulnerabilities.

As mentioned in the source code:

// Disable DTD and prevent XXE attacks.  See https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Prevention_Cheat_Sheet#libxerces-c for additional details.
m_parser->setIgnoreCachedDTD(true);
m_parser->setSkipDTDValidation(true);
m_parser->setCreateEntityReferenceNodes(false);

The setCreateEntityReferenceNodes method should be called with true as argument and not false, this way the entity reference nodes are created in the DOM instead of replacing the entity references with their content like today.

The Owasp recommendation for xerces was updated recently, so you might be interested in fixing the code.

I was not able to exploit this potential XXE vulnerability, I think the risk is very low, whihch is why I took the opportunity to report it here.

PS: This finding has been automatically found by SonarSource static source code analyzer products.

Eric