You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ws.apache.org by co...@apache.org on 2026/09/22 10:29:43 UTC
(ws-wss4j) branch coheigea/xop created (now 5515a6362)
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a change to branch coheigea/xop
in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
at 5515a6362 Xop hardening fix
This branch includes the following new commits:
new 5515a6362 Xop hardening fix
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(ws-wss4j) 01/01: Xop hardening fix
Posted by co...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch coheigea/xop
in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
commit 5515a63622a2a49551add7c98ff996b4759ed2aa
Author: Colm O hEigeartaigh <co...@apache.org>
AuthorDate: Tue Sep 22 11:29:22 2026 +0100
Xop hardening fix
---
.../apache/wss4j/common/util/AttachmentUtils.java | 10 ++-
.../wss4j/common/util/AttachmentUtilsTest.java | 80 ++++++++++++++++++++++
.../org/apache/wss4j/dom/util/WSSecurityUtil.java | 6 +-
.../apache/wss4j/dom/util/WSSecurityUtilTest.java | 58 ++++++++++++++++
4 files changed, 152 insertions(+), 2 deletions(-)
diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/util/AttachmentUtils.java b/ws-security-common/src/main/java/org/apache/wss4j/common/util/AttachmentUtils.java
index 50febaca5..0432678fe 100644
--- a/ws-security-common/src/main/java/org/apache/wss4j/common/util/AttachmentUtils.java
+++ b/ws-security-common/src/main/java/org/apache/wss4j/common/util/AttachmentUtils.java
@@ -645,9 +645,17 @@ public final class AttachmentUtils {
}
public static String getAttachmentId(String xopUri) throws WSSecurityException {
+ if (xopUri == null || !xopUri.startsWith("cid:")) {
+ throw new WSSecurityException(
+ WSSecurityException.ErrorCode.INVALID_SECURITY,
+ "empty", new Object[] {"Not an attachment reference: " + xopUri}
+ );
+ }
try {
return URLDecoder.decode(xopUri.substring("cid:".length()), StandardCharsets.UTF_8.name());
- } catch (UnsupportedEncodingException e) {
+ } catch (UnsupportedEncodingException | IllegalArgumentException e) {
+ // URLDecoder raises IllegalArgumentException for a malformed escape - "cid:%" say -
+ // which is unchecked, and so would otherwise leave WSS4J as one.
throw new WSSecurityException(
WSSecurityException.ErrorCode.INVALID_SECURITY,
"empty", new Object[] {"Attachment ID cannot be decoded: " + xopUri}
diff --git a/ws-security-common/src/test/java/org/apache/wss4j/common/util/AttachmentUtilsTest.java b/ws-security-common/src/test/java/org/apache/wss4j/common/util/AttachmentUtilsTest.java
new file mode 100644
index 000000000..7d07bfc6d
--- /dev/null
+++ b/ws-security-common/src/test/java/org/apache/wss4j/common/util/AttachmentUtilsTest.java
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.wss4j.common.util;
+
+import org.apache.wss4j.common.ext.WSSecurityException;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * The attachment URI reaches getAttachmentId from the wire at most call sites, so every shape it
+ * can take has to come back as a WSSecurityException rather than an unchecked one.
+ */
+public class AttachmentUtilsTest {
+
+ @Test
+ public void testAttachmentIdIsDecoded() throws Exception {
+ assertEquals("attachment", AttachmentUtils.getAttachmentId("cid:attachment"));
+ assertEquals("a b", AttachmentUtils.getAttachmentId("cid:a+b"));
+ assertEquals("a b", AttachmentUtils.getAttachmentId("cid:a%20b"));
+ assertEquals("", AttachmentUtils.getAttachmentId("cid:"));
+ }
+
+ @Test
+ public void testUriShorterThanTheCidPrefixIsRejected() {
+ // "".substring(4) is a StringIndexOutOfBoundsException, not a security fault
+ for (String uri : new String[] {"", "c", "ci", "cid"}) {
+ WSSecurityException ex =
+ assertThrows(WSSecurityException.class, () -> AttachmentUtils.getAttachmentId(uri),
+ "Expected a rejection of " + "\"" + uri + "\"");
+ assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY, ex.getErrorCode());
+ }
+ }
+
+ @Test
+ public void testUriThatIsNotAnAttachmentReferenceIsRejected() {
+ for (String uri : new String[] {"#foo", "http://example.com/a", "CID:attachment", " cid:a"}) {
+ WSSecurityException ex =
+ assertThrows(WSSecurityException.class, () -> AttachmentUtils.getAttachmentId(uri),
+ "Expected a rejection of " + uri);
+ assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY, ex.getErrorCode());
+ }
+ }
+
+ @Test
+ public void testNullUriIsRejected() {
+ WSSecurityException ex =
+ assertThrows(WSSecurityException.class, () -> AttachmentUtils.getAttachmentId(null));
+ assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY, ex.getErrorCode());
+ }
+
+ @Test
+ public void testMalformedEscapeIsRejected() {
+ // URLDecoder raises IllegalArgumentException for these, which is unchecked
+ for (String uri : new String[] {"cid:%", "cid:%zz", "cid:a%2"}) {
+ WSSecurityException ex =
+ assertThrows(WSSecurityException.class, () -> AttachmentUtils.getAttachmentId(uri),
+ "Expected a rejection of " + uri);
+ assertEquals(WSSecurityException.ErrorCode.INVALID_SECURITY, ex.getErrorCode());
+ }
+ }
+}
diff --git a/ws-security-dom/src/main/java/org/apache/wss4j/dom/util/WSSecurityUtil.java b/ws-security-dom/src/main/java/org/apache/wss4j/dom/util/WSSecurityUtil.java
index 1496a740e..264720ea5 100644
--- a/ws-security-dom/src/main/java/org/apache/wss4j/dom/util/WSSecurityUtil.java
+++ b/ws-security-dom/src/main/java/org/apache/wss4j/dom/util/WSSecurityUtil.java
@@ -630,7 +630,11 @@ public final class WSSecurityUtil {
boolean removeAttachments) throws WSSecurityException {
for (Element includeElement : includeElements) {
String xopURI = includeElement.getAttributeNS(null, "href");
- if (xopURI != null) {
+ // getAttributeNS answers an absent attribute with "", never null, so the old null
+ // check admitted an xop:Include carrying no href at all. Test for an attachment
+ // reference the way the other xop:Include call sites do, and leave anything else
+ // alone: an Include that names no attachment has no attachment bytes to inline.
+ if (xopURI.startsWith("cid:")) {
// Retrieve the attachment bytes
byte[] attachmentBytes =
WSSecurityUtil.getBytesFromAttachment(xopURI, attachmentCallbackHandler, removeAttachments);
diff --git a/ws-security-dom/src/test/java/org/apache/wss4j/dom/util/WSSecurityUtilTest.java b/ws-security-dom/src/test/java/org/apache/wss4j/dom/util/WSSecurityUtilTest.java
index 5005dec8f..80e748693 100644
--- a/ws-security-dom/src/test/java/org/apache/wss4j/dom/util/WSSecurityUtilTest.java
+++ b/ws-security-dom/src/test/java/org/apache/wss4j/dom/util/WSSecurityUtilTest.java
@@ -19,15 +19,25 @@
package org.apache.wss4j.dom.util;
+import java.util.Collections;
import java.util.List;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.callback.UnsupportedCallbackException;
+
import org.apache.wss4j.common.ConfigurationConstants;
+import org.apache.wss4j.common.ext.WSSecurityException;
+import org.apache.wss4j.common.util.SOAPUtil;
import org.apache.wss4j.dom.WSConstants;
import org.apache.wss4j.dom.engine.WSSConfig;
import org.apache.wss4j.dom.handler.HandlerAction;
import org.junit.jupiter.api.Test;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
public class WSSecurityUtilTest {
@@ -103,4 +113,52 @@ public class WSSecurityUtilTest {
assertEquals(WSConstants.SIGN, decodeActions.get(0).getAction());
assertEquals(WSConstants.DKT_ENCR, decodeActions.get(1).getAction());
}
+
+ /**
+ * An xop:Include that names no attachment has no attachment bytes to inline, so it is left
+ * as it is. It used to be read as an attachment reference regardless - getAttributeNS
+ * answers an absent attribute with "", never null - and "" parsed as an attachment id threw
+ * StringIndexOutOfBoundsException out of the engine.
+ */
+ @Test
+ public void testInlineAttachmentsIgnoresIncludeThatNamesNoAttachment() throws Exception {
+ for (String href : new String[] {null, "", "x", "#foo", "http://example.com/a"}) {
+ Document doc = SOAPUtil.toSOAPPart(SOAPUtil.SAMPLE_SOAP_MSG);
+ Element include = doc.createElementNS(WSConstants.XOP_NS, "xop:Include");
+ if (href != null) {
+ include.setAttributeNS(null, "href", href);
+ }
+ doc.getDocumentElement().appendChild(include);
+
+ WSSecurityUtil.inlineAttachments(
+ Collections.singletonList(include), rejectingAttachmentCallbackHandler(), true);
+
+ assertSame(include, doc.getDocumentElement().getLastChild(),
+ "The Include should have been left alone for href " + href);
+ }
+ }
+
+ /**
+ * An Include that does name an attachment is still resolved, and a failure to resolve it is
+ * reported as a WSSecurityException.
+ */
+ @Test
+ public void testInlineAttachmentsResolvesCidHref() throws Exception {
+ Document doc = SOAPUtil.toSOAPPart(SOAPUtil.SAMPLE_SOAP_MSG);
+ Element include = doc.createElementNS(WSConstants.XOP_NS, "xop:Include");
+ include.setAttributeNS(null, "href", "cid:missing");
+ doc.getDocumentElement().appendChild(include);
+
+ assertThrows(WSSecurityException.class,
+ () -> WSSecurityUtil.inlineAttachments(
+ Collections.singletonList(include), rejectingAttachmentCallbackHandler(), true));
+ }
+
+ private static CallbackHandler rejectingAttachmentCallbackHandler() {
+ // Non-null, so that getBytesFromAttachment gets as far as parsing the URI
+ return callbacks -> {
+ throw new UnsupportedCallbackException(callbacks[0]);
+ };
+ }
+
}
\ No newline at end of file