diff --git a/src/main/java/org/openrewrite/java/security/xml/DBFInsertPropertyStatementVisitor.java b/src/main/java/org/openrewrite/java/security/xml/DBFInsertPropertyStatementVisitor.java index d67d2e9..c2962c6 100644 --- a/src/main/java/org/openrewrite/java/security/xml/DBFInsertPropertyStatementVisitor.java +++ b/src/main/java/org/openrewrite/java/security/xml/DBFInsertPropertyStatementVisitor.java @@ -27,6 +27,7 @@ public class DBFInsertPropertyStatementVisitor
extends XmlFactoryInsertVisito private final boolean disallowGeneralEntities; private final boolean disallowParameterEntities; private final boolean disallowLoadExternalDTD; + private final boolean setXIncludeAware; public DBFInsertPropertyStatementVisitor( J.Block scope, @@ -35,8 +36,9 @@ public DBFInsertPropertyStatementVisitor( boolean needsDisallowDoctypesTrue, boolean needsDisableGeneralEntities, boolean needsDisableParameterEntities, - boolean needsLoadExternalDTD - ) { + boolean needsLoadExternalDTD, + boolean needsSetXIncludeAware, + boolean needsSetExpandEntityReferences) { super( scope, dbfFactoryVariable, @@ -50,27 +52,32 @@ public DBFInsertPropertyStatementVisitor( disallowGeneralEntities = false; disallowParameterEntities = false; disallowLoadExternalDTD = false; + setXIncludeAware = false; } else if (needsDisallowDoctypesTrue && !accIsEmpty) { disallowDoctypes = false; disallowGeneralEntities = needsDisableGeneralEntities; disallowParameterEntities = needsDisableParameterEntities; disallowLoadExternalDTD = needsLoadExternalDTD; + setXIncludeAware = needsSetXIncludeAware; } else if (!needsDisallowDoctypesTrue && !accIsEmpty) { disallowDoctypes = false; disallowGeneralEntities = false; disallowLoadExternalDTD = false; disallowParameterEntities = false; + setXIncludeAware = false; } else { disallowDoctypes = false; disallowGeneralEntities = false; disallowLoadExternalDTD = false; disallowParameterEntities = false; + setXIncludeAware = false; } + } @Override public void updateTemplate() { - if (disallowDoctypes && !disallowGeneralEntities && !disallowParameterEntities && !disallowLoadExternalDTD) { + if (disallowDoctypes && !disallowGeneralEntities && !disallowParameterEntities && !disallowLoadExternalDTD && !setXIncludeAware) { getTemplate().append( "String FEATURE = \"http://apache.org/xml/features/disallow-doctype-decl\";\n" + "try {\n" + @@ -92,17 +99,32 @@ public void updateTemplate() { "\n" + " FEATURE = \"http://xml.org/sax/features/external-general-entities\";\n" + " " + getFactoryVariableName() + ".setFeature(FEATURE, false);\n" + - "\n" + - " " + getFactoryVariableName() + ".setXIncludeAware(false);\n" + - " " + getFactoryVariableName() + ".setExpandEntityReferences(false);\n" + - "\n" + - " " + getFactoryVariableName() + ".setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);\n" + - "\n" + - "} catch (ParserConfigurationException e) {\n" + - " throw new IllegalStateException(\"The feature '\"\n" + - " + FEATURE + \"' is not supported by your XML processor.\", e);\n" + - "}\n" + "\n" ); + if (setXIncludeAware){ + getTemplate().append( + " " + getFactoryVariableName() + ".setXIncludeAware(false);\n" + + " " + getFactoryVariableName() + ".setExpandEntityReferences(false);\n" + + "\n" + + " " + getFactoryVariableName() + ".setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);\n" + + "\n" + + "} catch (ParserConfigurationException e) {\n" + + " throw new IllegalStateException(\"The feature '\"\n" + + " + FEATURE + \"' is not supported by your XML processor.\", e);\n" + + "}\n" + ); + } else { + getTemplate().append( + " " + getFactoryVariableName() + ".setExpandEntityReferences(false);\n" + + "\n" + + " " + getFactoryVariableName() + ".setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);\n" + + "\n" + + "} catch (ParserConfigurationException e) {\n" + + " throw new IllegalStateException(\"The feature '\"\n" + + " + FEATURE + \"' is not supported by your XML processor.\", e);\n" + + "}\n" + ); + } } } } diff --git a/src/main/java/org/openrewrite/java/security/xml/DocumentBuilderFactoryFixVisitor.java b/src/main/java/org/openrewrite/java/security/xml/DocumentBuilderFactoryFixVisitor.java index 2c4bde3..79bf94b 100644 --- a/src/main/java/org/openrewrite/java/security/xml/DocumentBuilderFactoryFixVisitor.java +++ b/src/main/java/org/openrewrite/java/security/xml/DocumentBuilderFactoryFixVisitor.java @@ -40,7 +40,8 @@ public class DocumentBuilderFactoryFixVisitor
extends XmlFactoryVisitor
{
static final InvocationMatcher DBF_NEW_INSTANCE = InvocationMatcher.fromMethodMatcher(new MethodMatcher("javax.xml.parsers.DocumentBuilderFactory newInstance*()"));
static final InvocationMatcher DBF_PARSER_SET_FEATURE = InvocationMatcher.fromMethodMatcher(new MethodMatcher("javax.xml.parsers.DocumentBuilderFactory setFeature(java.lang.String, boolean)"));
-
+ static final InvocationMatcher DBF_PARSER_SET_X_INCLUDE_AWARE = InvocationMatcher.fromMethodMatcher(new MethodMatcher("javax.xml.parsers.DocumentBuilderFactory setXIncludeAware(boolean)"));
+ static final InvocationMatcher DBF_PARSER_SET_EXPAND_ENTITY_REFERENCES = InvocationMatcher.fromMethodMatcher(new MethodMatcher("javax.xml.parsers.DocumentBuilderFactory setExpandEntityReferences(boolean)"));
private static final String DBF_FQN = "javax.xml.parsers.DocumentBuilderFactory";
private static final String DISALLOW_DOCTYPE_DECLARATIONS = "http://apache.org/xml/features/disallow-doctype-decl";
private static final String DISABLE_GENERAL_ENTITIES = "http://xml.org/sax/features/external-general-entities";
@@ -127,6 +128,25 @@ public Expression visitExpression(Expression expression, P p) {
return super.visitExpression(expression, p);
}
+ @Override
+ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, P ctx) {
+ J.MethodInvocation mi = super.visitMethodInvocation(method, ctx);
+ if (DBF_PARSER_SET_X_INCLUDE_AWARE.matches(mi)) {
+ addMessage(SET_X_INCLUDE_AWARE_PROPERTY_NAME);
+ } else if (DBF_PARSER_SET_EXPAND_ENTITY_REFERENCES.matches(mi)){
+ addMessage(SET_EXPAND_ENTITY_REFERENCES_PROPERTY_NAME);
+ }
+ return mi;
+ }
+
+ @Override
+ public J.Block visitBlock(J.Block block, P p) {
+ if (J.Block.isInitBlock(getCursor())) {
+ addMessage(DBF_INITIALIZATION_METHOD);
+ }
+ return super.visitBlock(block, p);
+ }
+
@Override
public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, P ctx) {
J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx);
@@ -138,6 +158,8 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, P
Cursor generalEntitiesDisabledCursor = getCursor().getMessage(DISABLE_GENERAL_ENTITIES + i);
Cursor parameterEntitiesDisabledCursor = getCursor().getMessage(DISABLE_PARAMETER_ENTITIES + i);
Cursor loadExternalDTDCursor = getCursor().getMessage(LOAD_EXTERNAL_DTD + i);
+ Cursor setXIncludeAwareCursor = getCursor().getMessage(SET_X_INCLUDE_AWARE_PROPERTY_NAME + i);
+ Cursor setExpandEntityReferencesCursor = getCursor().getMessage(SET_EXPAND_ENTITY_REFERENCES_PROPERTY_NAME + i);
Cursor setPropertyBlockCursor = null;
if (disallowedDTDTrueCursor == null) {
@@ -154,8 +176,9 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, P
disallowedDTDTrueCursor == null,
generalEntitiesDisabledCursor == null,
parameterEntitiesDisabledCursor == null,
- loadExternalDTDCursor == null
- ));
+ loadExternalDTDCursor == null,
+ setXIncludeAwareCursor == null,
+ setExpandEntityReferencesCursor == null));
}
}
return cd;
diff --git a/src/test/java/org/openrewrite/java/security/xml/DocumentBuilderFactoryXXETest.java b/src/test/java/org/openrewrite/java/security/xml/DocumentBuilderFactoryXXETest.java
index 92fa8cc..d7075c6 100644
--- a/src/test/java/org/openrewrite/java/security/xml/DocumentBuilderFactoryXXETest.java
+++ b/src/test/java/org/openrewrite/java/security/xml/DocumentBuilderFactoryXXETest.java
@@ -277,4 +277,85 @@ class myDBFReader {
)
);
}
+
+ @Test
+ void factoryIsNotVulnerableWithDTDsAndXIncludeAwareButPropertiesMissing() {
+ rewriteRun(
+ xml(
+ """
+
+
+
+ ]>
+