From 57cd9208c4b8a523934024654bcd40238a1857dc Mon Sep 17 00:00:00 2001 From: Ian Nesbitt Date: Fri, 10 Jul 2026 12:35:24 -0700 Subject: [PATCH 01/11] adding `Valve` workaround for GHSA-95v2-fvxr-qg83 with implementation suggestion for metacat<3.5.0 --- .../dataone/security/TemporaryMitigationValve | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 src/main/java/org/dataone/security/TemporaryMitigationValve diff --git a/src/main/java/org/dataone/security/TemporaryMitigationValve b/src/main/java/org/dataone/security/TemporaryMitigationValve new file mode 100644 index 0000000..ca9d5c9 --- /dev/null +++ b/src/main/java/org/dataone/security/TemporaryMitigationValve @@ -0,0 +1,74 @@ +package org.dataone.security; + +import java.io.IOException; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.regex.Pattern; + +import javax.servlet.ServletException; + +import org.apache.catalina.Valve; +import org.apache.catalina.connector.Request; +import org.apache.catalina.connector.Response; +import org.apache.catalina.valves.ValveBase; + +/** + * Temporary mitigation Valve to handle GHSA-95v2-fvxr-qg83-style path confusion/bypass attempts. + * Added 2026-07-10. + * Enable via the element under in server.xml, e.g.: + * + */ +public class TemporaryMitigationValve extends ValveBase { + + private static final int MAX_DECODE_ROUNDS = 3; + + // deny patterns + private static final Pattern SUSPICIOUS = Pattern.compile( + "(?i)(\\.{2}|%2e|%2f|%5c|\\\\|/\\./|/\\.\\./|%252e|%252f|%255c)" + ); + + @Override + public void invoke(Request request, Response response) throws IOException, ServletException { + String uri = safe(request.getRequestURI()); + String qs = request.getQueryString(); + String target = (qs == null) ? uri : (uri + "?" + qs); + + if (isSuspicious(target)) { + response.sendError(400, "Malformed request target"); + return; + } + + Valve next = getNext(); + if (next != null) { + next.invoke(request, response); + } + } + + private boolean isSuspicious(String input) { + String current = input; + for (int i = 0; i < MAX_DECODE_ROUNDS; i++) { + if (SUSPICIOUS.matcher(current).find()) { + return true; + } + String decoded = decodeOnce(current); + if (decoded.equals(current)) { + break; + } + current = decoded; + } + return SUSPICIOUS.matcher(current).find(); + } + + private String decodeOnce(String value) { + try { + return URLDecoder.decode(value, StandardCharsets.UTF_8); + } catch (IllegalArgumentException e) { + // Invalid %-encoding => treat as suspicious + return "%BAD_ENCODING%"; + } + } + + private String safe(String v) { + return v == null ? "" : v; + } +} \ No newline at end of file From 896e35966a0050c0417642cafeba985f40900b80 Mon Sep 17 00:00:00 2001 From: Ian Nesbitt Date: Fri, 10 Jul 2026 12:58:01 -0700 Subject: [PATCH 02/11] correcting filename --- .../{TemporaryMitigationValve => TemporaryMitigationValve.java} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/main/java/org/dataone/security/{TemporaryMitigationValve => TemporaryMitigationValve.java} (100%) diff --git a/src/main/java/org/dataone/security/TemporaryMitigationValve b/src/main/java/org/dataone/security/TemporaryMitigationValve.java similarity index 100% rename from src/main/java/org/dataone/security/TemporaryMitigationValve rename to src/main/java/org/dataone/security/TemporaryMitigationValve.java From c88caa382acaa3951d6939b964b1ceebcc5ec3c4 Mon Sep 17 00:00:00 2001 From: Ian Nesbitt Date: Fri, 10 Jul 2026 13:00:12 -0700 Subject: [PATCH 03/11] adding license statement --- .../security/TemporaryMitigationValve.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/main/java/org/dataone/security/TemporaryMitigationValve.java b/src/main/java/org/dataone/security/TemporaryMitigationValve.java index ca9d5c9..cb1adde 100644 --- a/src/main/java/org/dataone/security/TemporaryMitigationValve.java +++ b/src/main/java/org/dataone/security/TemporaryMitigationValve.java @@ -1,3 +1,25 @@ +/** + * 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 java.io.IOException; From 347f2d739c734c1157c4dcd1babaf0bc48713335 Mon Sep 17 00:00:00 2001 From: Ian Nesbitt Date: Fri, 10 Jul 2026 13:08:23 -0700 Subject: [PATCH 04/11] addressing suggestion not to return a magic string but instead return `null` when an illegal sequence is found --- .../org/dataone/security/TemporaryMitigationValve.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/dataone/security/TemporaryMitigationValve.java b/src/main/java/org/dataone/security/TemporaryMitigationValve.java index cb1adde..dfa4f74 100644 --- a/src/main/java/org/dataone/security/TemporaryMitigationValve.java +++ b/src/main/java/org/dataone/security/TemporaryMitigationValve.java @@ -73,6 +73,9 @@ private boolean isSuspicious(String input) { return true; } String decoded = decodeOnce(current); + if (decoded == null) { + return true; + } if (decoded.equals(current)) { break; } @@ -85,8 +88,8 @@ private String decodeOnce(String value) { try { return URLDecoder.decode(value, StandardCharsets.UTF_8); } catch (IllegalArgumentException e) { - // Invalid %-encoding => treat as suspicious - return "%BAD_ENCODING%"; + // Invalid %-encoding: signal decode failure to caller. + return null; } } From 705c789917b829fb72e46c407db5823be79f7f12 Mon Sep 17 00:00:00 2001 From: Ian Nesbitt Date: Fri, 10 Jul 2026 13:21:14 -0700 Subject: [PATCH 05/11] adding logging --- .../java/org/dataone/security/TemporaryMitigationValve.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/org/dataone/security/TemporaryMitigationValve.java b/src/main/java/org/dataone/security/TemporaryMitigationValve.java index dfa4f74..ec4456e 100644 --- a/src/main/java/org/dataone/security/TemporaryMitigationValve.java +++ b/src/main/java/org/dataone/security/TemporaryMitigationValve.java @@ -33,6 +33,8 @@ import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; import org.apache.catalina.valves.ValveBase; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; /** * Temporary mitigation Valve to handle GHSA-95v2-fvxr-qg83-style path confusion/bypass attempts. @@ -42,6 +44,8 @@ */ public class TemporaryMitigationValve extends ValveBase { + private static final Logger logger = LogManager.getLogger(TemporaryMitigationValve.class.getName()); + private static final int MAX_DECODE_ROUNDS = 3; // deny patterns @@ -56,6 +60,8 @@ public void invoke(Request request, Response response) throws IOException, Servl String target = (qs == null) ? uri : (uri + "?" + qs); if (isSuspicious(target)) { + logger.warn("Rejecting suspicious request from remote address {} for URI {}", + safe(request.getRemoteAddr()), uri); response.sendError(400, "Malformed request target"); return; } From f7c782529d4278b21e8cba973acdf26209270359 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:25:50 +0000 Subject: [PATCH 06/11] test: add focused TemporaryMitigationValve regression coverage --- .../TemporaryMitigationValveTest.java | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 src/test/java/org/dataone/security/TemporaryMitigationValveTest.java diff --git a/src/test/java/org/dataone/security/TemporaryMitigationValveTest.java b/src/test/java/org/dataone/security/TemporaryMitigationValveTest.java new file mode 100644 index 0000000..bae86dc --- /dev/null +++ b/src/test/java/org/dataone/security/TemporaryMitigationValveTest.java @@ -0,0 +1,73 @@ +/** + * 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.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +import org.junit.Test; + +public class TemporaryMitigationValveTest { + + private final TemporaryMitigationValve valve = new TemporaryMitigationValve(); + + @Test + public void rejectsPlainTraversal() { + assertTrue(isSuspicious("/v2/resolve/../etc/passwd")); + } + + @Test + public void rejectsSingleEncodedTraversal() { + assertTrue(isSuspicious("/v2/resolve/%2e%2e%2fetc/passwd")); + } + + @Test + public void rejectsDoubleEncodedTraversal() { + assertTrue(isSuspicious("/v2/resolve/%252e%252e%252fetc/passwd")); + } + + @Test + public void rejectsMalformedPercentEncoding() { + assertTrue(isSuspicious("/v2/resolve/%ZZ")); + } + + @Test + public void allowsNormalRequestTarget() { + assertFalse(isSuspicious("/v2/resolve/abc123?includeMetadata=true")); + } + + private boolean isSuspicious(String input) { + try { + Method method = TemporaryMitigationValve.class.getDeclaredMethod("isSuspicious", String.class); + method.setAccessible(true); + return (Boolean) method.invoke(valve, input); + } catch (NoSuchMethodException | IllegalAccessException e) { + throw new AssertionError("Failed to access TemporaryMitigationValve.isSuspicious", e); + } catch (InvocationTargetException e) { + throw new AssertionError("Unexpected exception from TemporaryMitigationValve.isSuspicious", e); + } + } +} From d0d106cadfb5bdd27efcec8abe309a833d0297e9 Mon Sep 17 00:00:00 2001 From: Ian Nesbitt Date: Fri, 10 Jul 2026 13:32:25 -0700 Subject: [PATCH 07/11] Implementing docstring suggestion for explicit code references Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../java/org/dataone/security/TemporaryMitigationValve.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/dataone/security/TemporaryMitigationValve.java b/src/main/java/org/dataone/security/TemporaryMitigationValve.java index ec4456e..a86e489 100644 --- a/src/main/java/org/dataone/security/TemporaryMitigationValve.java +++ b/src/main/java/org/dataone/security/TemporaryMitigationValve.java @@ -39,8 +39,8 @@ /** * Temporary mitigation Valve to handle GHSA-95v2-fvxr-qg83-style path confusion/bypass attempts. * Added 2026-07-10. - * Enable via the element under in server.xml, e.g.: - * + * Enable via the {@code } element under {@code } in {@code server.xml}, e.g.: + * {@code } */ public class TemporaryMitigationValve extends ValveBase { From d7f6b698204eab7faf7fe02e2337a84dbf8f0645 Mon Sep 17 00:00:00 2001 From: Ian Nesbitt Date: Mon, 13 Jul 2026 06:25:47 -0700 Subject: [PATCH 08/11] changing security module to exclude external DTD declarations and entity declarations in MIME multipart data, and adding accompanying tests --- .../security/TemporaryMitigationValve.java | 105 ---------- .../security/XmlSecurityValidationValve.java | 188 ++++++++++++++++++ .../TemporaryMitigationValveTest.java | 73 ------- .../XmlSecurityValidationValveTest.java | 109 ++++++++++ 4 files changed, 297 insertions(+), 178 deletions(-) delete mode 100644 src/main/java/org/dataone/security/TemporaryMitigationValve.java create mode 100644 src/main/java/org/dataone/security/XmlSecurityValidationValve.java delete mode 100644 src/test/java/org/dataone/security/TemporaryMitigationValveTest.java create mode 100644 src/test/java/org/dataone/security/XmlSecurityValidationValveTest.java diff --git a/src/main/java/org/dataone/security/TemporaryMitigationValve.java b/src/main/java/org/dataone/security/TemporaryMitigationValve.java deleted file mode 100644 index a86e489..0000000 --- a/src/main/java/org/dataone/security/TemporaryMitigationValve.java +++ /dev/null @@ -1,105 +0,0 @@ -/** - * 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 java.io.IOException; -import java.net.URLDecoder; -import java.nio.charset.StandardCharsets; -import java.util.regex.Pattern; - -import javax.servlet.ServletException; - -import org.apache.catalina.Valve; -import org.apache.catalina.connector.Request; -import org.apache.catalina.connector.Response; -import org.apache.catalina.valves.ValveBase; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; - -/** - * Temporary mitigation Valve to handle GHSA-95v2-fvxr-qg83-style path confusion/bypass attempts. - * Added 2026-07-10. - * Enable via the {@code } element under {@code } in {@code server.xml}, e.g.: - * {@code } - */ -public class TemporaryMitigationValve extends ValveBase { - - private static final Logger logger = LogManager.getLogger(TemporaryMitigationValve.class.getName()); - - private static final int MAX_DECODE_ROUNDS = 3; - - // deny patterns - private static final Pattern SUSPICIOUS = Pattern.compile( - "(?i)(\\.{2}|%2e|%2f|%5c|\\\\|/\\./|/\\.\\./|%252e|%252f|%255c)" - ); - - @Override - public void invoke(Request request, Response response) throws IOException, ServletException { - String uri = safe(request.getRequestURI()); - String qs = request.getQueryString(); - String target = (qs == null) ? uri : (uri + "?" + qs); - - if (isSuspicious(target)) { - logger.warn("Rejecting suspicious request from remote address {} for URI {}", - safe(request.getRemoteAddr()), uri); - response.sendError(400, "Malformed request target"); - return; - } - - Valve next = getNext(); - if (next != null) { - next.invoke(request, response); - } - } - - private boolean isSuspicious(String input) { - String current = input; - for (int i = 0; i < MAX_DECODE_ROUNDS; i++) { - if (SUSPICIOUS.matcher(current).find()) { - return true; - } - String decoded = decodeOnce(current); - if (decoded == null) { - return true; - } - if (decoded.equals(current)) { - break; - } - current = decoded; - } - return SUSPICIOUS.matcher(current).find(); - } - - private String decodeOnce(String value) { - try { - return URLDecoder.decode(value, StandardCharsets.UTF_8); - } catch (IllegalArgumentException e) { - // Invalid %-encoding: signal decode failure to caller. - return null; - } - } - - private String safe(String v) { - return v == null ? "" : v; - } -} \ No newline at end of file diff --git a/src/main/java/org/dataone/security/XmlSecurityValidationValve.java b/src/main/java/org/dataone/security/XmlSecurityValidationValve.java new file mode 100644 index 0000000..080b9df --- /dev/null +++ b/src/main/java/org/dataone/security/XmlSecurityValidationValve.java @@ -0,0 +1,188 @@ +/** + * 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 xml.parsers.SAXParser; +import 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 handle GHSA-95v2-fvxr-qg83-style path confusion/bypass attempts. + * Added 2026-07-10. + * Enable via the element under in server.xml, e.g.: + * + */ +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 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 && (contentType.contains("text/xml") || contentType.contains("application/xml"))) { + return true; + } + return fileName != null && fileName.toLowerCase().endsWith(".xml"); + } + + 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); + + 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; + } + } + +} diff --git a/src/test/java/org/dataone/security/TemporaryMitigationValveTest.java b/src/test/java/org/dataone/security/TemporaryMitigationValveTest.java deleted file mode 100644 index bae86dc..0000000 --- a/src/test/java/org/dataone/security/TemporaryMitigationValveTest.java +++ /dev/null @@ -1,73 +0,0 @@ -/** - * 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.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; - -import org.junit.Test; - -public class TemporaryMitigationValveTest { - - private final TemporaryMitigationValve valve = new TemporaryMitigationValve(); - - @Test - public void rejectsPlainTraversal() { - assertTrue(isSuspicious("/v2/resolve/../etc/passwd")); - } - - @Test - public void rejectsSingleEncodedTraversal() { - assertTrue(isSuspicious("/v2/resolve/%2e%2e%2fetc/passwd")); - } - - @Test - public void rejectsDoubleEncodedTraversal() { - assertTrue(isSuspicious("/v2/resolve/%252e%252e%252fetc/passwd")); - } - - @Test - public void rejectsMalformedPercentEncoding() { - assertTrue(isSuspicious("/v2/resolve/%ZZ")); - } - - @Test - public void allowsNormalRequestTarget() { - assertFalse(isSuspicious("/v2/resolve/abc123?includeMetadata=true")); - } - - private boolean isSuspicious(String input) { - try { - Method method = TemporaryMitigationValve.class.getDeclaredMethod("isSuspicious", String.class); - method.setAccessible(true); - return (Boolean) method.invoke(valve, input); - } catch (NoSuchMethodException | IllegalAccessException e) { - throw new AssertionError("Failed to access TemporaryMitigationValve.isSuspicious", e); - } catch (InvocationTargetException e) { - throw new AssertionError("Unexpected exception from TemporaryMitigationValve.isSuspicious", e); - } - } -} diff --git a/src/test/java/org/dataone/security/XmlSecurityValidationValveTest.java b/src/test/java/org/dataone/security/XmlSecurityValidationValveTest.java new file mode 100644 index 0000000..a3256df --- /dev/null +++ b/src/test/java/org/dataone/security/XmlSecurityValidationValveTest.java @@ -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 = + "" + + "]>" + + "&xxe;"; + + assertTrue(containsForbiddenXmlStructures(streamOf(maliciousXml))); + } + + @Test + public void rejectsXmlWithExternalDtdDeclaration() { + String maliciousXml = + "" + + "" + + "ok"; + + assertTrue(containsForbiddenXmlStructures(streamOf(maliciousXml))); + } + + @Test + public void allowsXmlWithoutDtdOrEntityDeclarations() { + String safeXml = "ok"; + 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)); + } +} From f1ab1eb2a56429215a1964dd84e587c0f14c2567 Mon Sep 17 00:00:00 2001 From: Ian Nesbitt Date: Mon, 13 Jul 2026 07:30:23 -0700 Subject: [PATCH 09/11] correcting imports Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../java/org/dataone/security/XmlSecurityValidationValve.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/dataone/security/XmlSecurityValidationValve.java b/src/main/java/org/dataone/security/XmlSecurityValidationValve.java index 080b9df..1de8e34 100644 --- a/src/main/java/org/dataone/security/XmlSecurityValidationValve.java +++ b/src/main/java/org/dataone/security/XmlSecurityValidationValve.java @@ -34,8 +34,8 @@ import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletResponse; -import xml.parsers.SAXParser; -import xml.parsers.SAXParserFactory; +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; From 267965e41e9d603f09e3ef2a95f9636d23fecaa8 Mon Sep 17 00:00:00 2001 From: Ian Nesbitt Date: Mon, 13 Jul 2026 07:31:03 -0700 Subject: [PATCH 10/11] correcting class name in docstring Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../org/dataone/security/XmlSecurityValidationValve.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/dataone/security/XmlSecurityValidationValve.java b/src/main/java/org/dataone/security/XmlSecurityValidationValve.java index 1de8e34..2cba8b7 100644 --- a/src/main/java/org/dataone/security/XmlSecurityValidationValve.java +++ b/src/main/java/org/dataone/security/XmlSecurityValidationValve.java @@ -47,10 +47,10 @@ import java.util.List; /** - * Temporary mitigation Valve to handle GHSA-95v2-fvxr-qg83-style path confusion/bypass attempts. + * Temporary mitigation Valve to reject multipart XML parts that declare DTDs/entities (XXE defense). * Added 2026-07-10. - * Enable via the element under in server.xml, e.g.: - * + * Enable via the {@code } element under {@code } in server.xml, e.g.: + * {@code } */ public class XmlSecurityValidationValve extends ValveBase { From 63bf0c65bb2d72b1f6760b5460f21e94eba26d2f Mon Sep 17 00:00:00 2001 From: Ian Nesbitt Date: Mon, 13 Jul 2026 07:34:30 -0700 Subject: [PATCH 11/11] avoiding case sensitivity issue Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../org/dataone/security/XmlSecurityValidationValve.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/dataone/security/XmlSecurityValidationValve.java b/src/main/java/org/dataone/security/XmlSecurityValidationValve.java index 2cba8b7..16155e5 100644 --- a/src/main/java/org/dataone/security/XmlSecurityValidationValve.java +++ b/src/main/java/org/dataone/security/XmlSecurityValidationValve.java @@ -123,8 +123,11 @@ public int doRead(ByteChunk chunk) throws IOException { } private boolean isXmlType(String contentType, String fileName) { - if (contentType != null && (contentType.contains("text/xml") || contentType.contains("application/xml"))) { - return true; + 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"); }