This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch 3_0_x-fixes
in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
The following commit(s) were added to refs/heads/3_0_x-fixes by this push:
new bb5aca733 Xop hardening fix (#728)
bb5aca733 is described below
commit bb5aca733bcf034ae2b55c1cd1facb198042c7e5
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Tue Sep 22 11:47:18 2026 +0100
Xop hardening fix (#728)
---
.../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
