diff --git a/openam-authentication/openam-auth-oauth2/src/main/java/org/forgerock/openam/authentication/modules/oauth2/OAuth.java b/openam-authentication/openam-auth-oauth2/src/main/java/org/forgerock/openam/authentication/modules/oauth2/OAuth.java index a2952659f0..b785a39bd3 100644 --- a/openam-authentication/openam-auth-oauth2/src/main/java/org/forgerock/openam/authentication/modules/oauth2/OAuth.java +++ b/openam-authentication/openam-auth-oauth2/src/main/java/org/forgerock/openam/authentication/modules/oauth2/OAuth.java @@ -23,7 +23,7 @@  * "Portions Copyrighted [year] [name of copyright owner]" * * Portions Copyrighted 2015 Nomura Research Institute, Ltd. - * Portions Copyrighted 2018-2025 3A Systems, LLC. + * Portions Copyrighted 2018-2026 3A Systems, LLC. */ package org.forgerock.openam.authentication.modules.oauth2; @@ -224,7 +224,7 @@ public int process2(Callback[] callbacks, int state) throws LoginException { String ProviderLogoutURL = config.getLogoutServiceUrl(); - String csrfStateTokenId = RandomStringUtils.randomAlphanumeric(32); + String csrfStateTokenId = newCsrfStateTokenId(); String csrfState = createAuthorizationState(); Token csrfStateToken = new Token(csrfStateTokenId, TokenType.GENERIC); csrfStateToken.setAttribute(CoreTokenField.STRING_ONE, csrfState); @@ -521,6 +521,15 @@ public int process2(Callback[] callbacks, int state) throws LoginException { throw new AuthLoginException(BUNDLE_NAME, "unknownState", null); } + /** + * Id of the CTS record holding the CSRF state; it also travels in the + * NONCE_TOKEN_ID cookie, so it is drawn from a cryptographic generator rather + * than the java.util.Random behind RandomStringUtils.randomAlphanumeric(). + */ + static String newCsrfStateTokenId() { + return RandomStringUtils.random(32, 0, 0, true, true, null, random); + } + private String createAuthorizationState() { return UUID.randomUUID().toString(); //new BigInteger(160, new SecureRandom()).toString(Character.MAX_RADIX); } diff --git a/openam-authentication/openam-auth-oauth2/src/test/java/org/forgerock/openam/authentication/modules/oauth2/OAuthTest.java b/openam-authentication/openam-auth-oauth2/src/test/java/org/forgerock/openam/authentication/modules/oauth2/OAuthTest.java index 6bc55aacab..c8f6b78232 100644 --- a/openam-authentication/openam-auth-oauth2/src/test/java/org/forgerock/openam/authentication/modules/oauth2/OAuthTest.java +++ b/openam-authentication/openam-auth-oauth2/src/test/java/org/forgerock/openam/authentication/modules/oauth2/OAuthTest.java @@ -1,7 +1,24 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ package org.forgerock.openam.authentication.modules.oauth2; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotEquals; +import static org.testng.Assert.assertTrue; import java.util.Collections; import java.util.HashMap; @@ -29,5 +46,13 @@ public void shouldRemoveCredentialAndAccountStatusAttributesFromUpdates() { assertFalse(attributes.containsKey("userPassword")); assertFalse(attributes.containsKey("inetuserstatus")); } + + @Test + public void csrfStateTokenIdIsThirtyTwoAlphanumericCharacters() { + String id = OAuth.newCsrfStateTokenId(); + + assertTrue(id.matches("[A-Za-z0-9]{32}"), id); + assertNotEquals(id, OAuth.newCsrfStateTokenId()); + } } diff --git a/openam-core/src/main/java/com/sun/identity/idm/plugins/files/FilesRepo.java b/openam-core/src/main/java/com/sun/identity/idm/plugins/files/FilesRepo.java index f48ed80b2f..3f96a56bd5 100755 --- a/openam-core/src/main/java/com/sun/identity/idm/plugins/files/FilesRepo.java +++ b/openam-core/src/main/java/com/sun/identity/idm/plugins/files/FilesRepo.java @@ -25,6 +25,7 @@ * $Id: FilesRepo.java,v 1.22 2008/07/02 17:21:21 kenwho Exp $ * * Portions Copyrighted 2011-2016 ForgeRock AS. + * Portions Copyrighted 2026 3A Systems LLC. */ package com.sun.identity.idm.plugins.files; @@ -1510,7 +1511,18 @@ void initDir(String rootDir) throws IdRepoException { } - File constructFile(String rootDir, IdType type, String name) { + File constructFile(String rootDir, IdType type, String name) + throws IdRepoException { + // The identity name becomes the file name: it must stay a single path + // component, or an identity could be created, read or deleted anywhere + // the server can write. Control characters are refused too, so a name + // cannot forge a log line. The name is caller data and is not logged. + if (!isSingleFileName(name)) { + debug.error("FilesRepo.constructFile: invalid identity name of type " + + type.getName() + ", length " + (name == null ? 0 : name.length())); + throw new IdRepoException(IdRepoBundle.getString(IdRepoErrorCode.ILLEGAL_ARGUMENTS), + IdRepoErrorCode.ILLEGAL_ARGUMENTS); + } // Construct file name File root = new File(rootDir); File subDir = new File(root, type.getName()); @@ -1701,6 +1713,25 @@ boolean containsAttrValue(Set attrValues, Set patterns) { return (false); } + /** + * Whether an identity name is usable as one file name under the type + * directory: not empty, not a dot directory, and free of path separators + * (both kinds, so a repository copied between platforms stays valid) and + * of control characters. + */ + static boolean isSingleFileName(String name) { + if (name == null || name.isEmpty() || name.equals(".") || name.equals("..")) { + return false; + } + for (int i = 0; i < name.length(); i++) { + char c = name.charAt(i); + if (c == '/' || c == '\\' || c < ' ' || c == 0x7f) { + return false; + } + } + return true; + } + // File name filter inner class class FileRepoFileFilter implements FilenameFilter { // Pattern to match @@ -1709,13 +1740,25 @@ class FileRepoFileFilter implements FilenameFilter { // Default constructor FileRepoFileFilter(String p) { if (p != null && p.length() != 0 && !p.equals("*")) { - // Replace "*" with ".*" + // "*" is the only wildcard; everything between wildcards is a + // literal, so quote it rather than letting the search pattern + // be interpreted as a regular expression. + StringBuilder regex = new StringBuilder(); + int from = 0; int idx = p.indexOf('*'); while (idx != -1) { - p = p.substring(0, idx) + ".*" + p.substring(idx + 1); - idx = p.indexOf('*', idx + 2); + if (idx > from) { + regex.append(Pattern.quote(p.substring(from, idx))); + } + regex.append(".*"); + from = idx + 1; + idx = p.indexOf('*', from); + } + if (from < p.length()) { + regex.append(Pattern.quote(p.substring(from))); } - pattern = Pattern.compile(p.toLowerCase()); + pattern = Pattern.compile(regex.toString(), + Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE); } } diff --git a/openam-core/src/test/java/com/sun/identity/idm/plugins/files/FilesRepoTest.java b/openam-core/src/test/java/com/sun/identity/idm/plugins/files/FilesRepoTest.java new file mode 100644 index 0000000000..22e7003217 --- /dev/null +++ b/openam-core/src/test/java/com/sun/identity/idm/plugins/files/FilesRepoTest.java @@ -0,0 +1,98 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.idm.plugins.files; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.File; + +import com.sun.identity.idm.IdRepoException; +import com.sun.identity.idm.IdType; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class FilesRepoTest { + + private final FilesRepo repo = new FilesRepo(); + + @Test + public void constructFileKeepsOrdinaryNamesUnderTheTypeDirectory() throws Exception { + File file = repo.constructFile("/var/openam/idRepo", IdType.USER, "demo.user-1"); + + assertThat(file).isEqualTo(new File(new File("/var/openam/idRepo", "user"), "demo.user-1")); + } + + @DataProvider + public Object[][] namesEscapingTheRepository() { + return new Object[][] { + {"../../../etc/passwd"}, + {".."}, + {"."}, + {"x/../../y"}, + {"sub/dir"}, + {"..\\..\\x"}, + {"evil\0"}, + {"evil\r\nforged"}, // CR/LF: log-line forging and never a valid name + {"tab\tname"}, + {""}, + {null}, + }; + } + + @Test(dataProvider = "namesEscapingTheRepository") + public void constructFileRejectsNamesThatLeaveTheTypeDirectory(String name) { + assertThatThrownBy(() -> repo.constructFile("/var/openam/idRepo", IdType.USER, name)) + .isInstanceOf(IdRepoException.class); + } + + @Test + public void fileFilterTreatsRegexMetaCharactersLiterally() { + FilesRepo.FileRepoFileFilter filter = repo.new FileRepoFileFilter("user.1"); + + assertThat(filter.accept(null, "user.1")).isTrue(); + assertThat(filter.accept(null, "userx1")).isFalse(); + } + + @Test + public void fileFilterStillExpandsWildcards() { + FilesRepo.FileRepoFileFilter filter = repo.new FileRepoFileFilter("us*r.1"); + + assertThat(filter.accept(null, "user.1")).isTrue(); + assertThat(filter.accept(null, "USER.1")).isTrue(); + assertThat(filter.accept(null, "uSomeThingr.1")).isTrue(); + assertThat(filter.accept(null, "userx1")).isFalse(); + assertThat(repo.new FileRepoFileFilter("*").accept(null, "anything")).isTrue(); + } + + @Test + public void fileFilterFoldsCaseBeyondAscii() { + FilesRepo.FileRepoFileFilter filter = repo.new FileRepoFileFilter("ЖОР*"); + + assertThat(filter.accept(null, "жора")).isTrue(); + assertThat(filter.accept(null, "Жора")).isTrue(); + assertThat(filter.accept(null, "жук")).isFalse(); + } + + @Test + public void fileFilterDoesNotInterpretInjectedRegex() { + // "(" would be a regex syntax error today; it must be a plain character. + FilesRepo.FileRepoFileFilter filter = repo.new FileRepoFileFilter("a(b"); + + assertThat(filter.accept(null, "a(b")).isTrue(); + assertThat(filter.accept(null, "ab")).isFalse(); + } +} diff --git a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/common/FSUtils.java b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/common/FSUtils.java index 8502f5d2a7..ce281b783a 100755 --- a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/common/FSUtils.java +++ b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/common/FSUtils.java @@ -358,6 +358,16 @@ public static void forwardRequest( } else { String resource = newUrl.substring( index + deploymentURI.length()); + // The target comes from the request (goto/RelayState). A forward + // bypasses the web.xml filters and can reach /WEB-INF, so refuse + // anything that is not a plain in-app path. + if (!ForwardPathValidator.isSafeForwardPath(resource)) { + FSUtils.debug.warning("FSUtils.forwardRequest: refusing to " + + "forward to a path with traversal or a reserved " + + "directory"); + response.sendError(HttpServletResponse.SC_BAD_REQUEST); + return; + } if (FSUtils.debug.messageEnabled()) { FSUtils.debug.message( "FSUtils.forwardRequest: Forwarding to :" + resource); diff --git a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/common/ForwardPathValidator.java b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/common/ForwardPathValidator.java new file mode 100644 index 0000000000..8ea68a44fa --- /dev/null +++ b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/common/ForwardPathValidator.java @@ -0,0 +1,213 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.federation.common; + +import java.io.ByteArrayOutputStream; +import java.nio.ByteBuffer; +import java.nio.charset.CharacterCodingException; +import java.nio.charset.CodingErrorAction; +import java.nio.charset.StandardCharsets; +import java.util.Locale; + +/** + * Sanity checks for request-derived values before they are handed to a + * {@code RequestDispatcher}. A forward is served from inside the web application: + * it can reach {@code /WEB-INF} and {@code /META-INF}, and it does not run the + * filters declared in {@code web.xml}, so a path an end user controls must never + * be allowed to climb out of the location the code intends to dispatch to. + *

+ * The container percent-decodes a dispatcher path once more and then collapses + * it ({@code //}, {@code /./}, {@code ;params}) before it maps it (Tomcat: + * {@code dispatchersUseEncodedPaths}, on by default), so every check runs on the + * decoded form as well as on the raw one, and the reserved-directory check reads + * the collapsed path. An escape that decodes to a URL delimiter ({@code %25}, + * {@code %3F}, {@code %23}) or to something that is not UTF-8 is refused + * outright: no in-app path of the product carries one. + */ +public final class ForwardPathValidator { + + private ForwardPathValidator() { + } + + /** + * Whether {@code path} may be forwarded to as-is: absolute, without {@code ..} + * segments in its raw or decoded form (path parameters stripped, as the + * container does), without backslashes, control characters, malformed or + * delimiter escapes, and not under a reserved directory once collapsed. + * + * @param path a context-relative path, optionally with a query string + * @return {@code true} if the path is safe to pass to a request dispatcher + */ + public static boolean isSafeForwardPath(String path) { + if (path == null || !path.startsWith("/")) { + return false; + } + int query = path.indexOf('?'); + String uri = query == -1 ? path : path.substring(0, query); + String resolved = resolve(uri); + if (resolved == null || isReserved(resolved)) { + return false; + } + // HttpServletRequest.getRequestDispatcher() drops a fragment before it + // maps the path, so the reserved-directory check has to see that form + // too; a ServletContext dispatcher keeps it, which the traversal checks + // above already cover. + int fragment = resolved.indexOf('#'); + return fragment == -1 || !isReserved(collapse(resolved.substring(0, fragment))); + } + + private static boolean isReserved(String collapsedPath) { + String lower = collapsedPath.toLowerCase(Locale.ROOT); + return lower.startsWith("/web-inf/") || lower.equals("/web-inf") + || lower.startsWith("/meta-inf/") || lower.equals("/meta-inf"); + } + + /** + * Whether {@code metaAlias} can be appended to a fixed handler path without + * changing which resource is dispatched to. + * + * @param metaAlias the provider meta alias taken from the request + * @return {@code true} if the alias contains no traversal + */ + public static boolean isSafeMetaAlias(String metaAlias) { + return metaAlias != null && !metaAlias.isEmpty() && resolve(metaAlias) != null; + } + + /** + * The path as the container will map it - decoded once and collapsed - or + * {@code null} when it must not be dispatched to: traversal, a backslash or a + * control character in either the raw or the decoded form, or an escape that + * is malformed, not UTF-8, or decodes to a URL delimiter. + */ + private static String resolve(String value) { + if (containsTraversal(value)) { + return null; + } + String decoded = decodeOnce(value); + if (decoded == null || containsTraversal(decoded)) { + return null; + } + return collapse(decoded); + } + + /** + * Collapses a path the way the container does before mapping it: {@code ;params} + * stripped from each segment, empty and {@code .} segments dropped. {@code ..} + * never reaches here. + */ + private static String collapse(String path) { + StringBuilder collapsed = new StringBuilder(path.length()); + for (String segment : path.split("/", -1)) { + int semicolon = segment.indexOf(';'); + if (semicolon != -1) { + segment = segment.substring(0, semicolon); + } + if (!segment.isEmpty() && !segment.equals(".")) { + collapsed.append('/').append(segment); + } + } + return collapsed.length() == 0 ? "/" : collapsed.toString(); + } + + private static boolean containsTraversal(String value) { + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + if (c == '\\' || c < ' ' || c == 0x7f) { + return true; + } + } + for (String segment : value.split("/", -1)) { + // The container strips ";param" from each segment before normalising. + int semicolon = segment.indexOf(';'); + if (semicolon != -1) { + segment = segment.substring(0, semicolon); + } + if (segment.equals("..")) { + return true; + } + } + return false; + } + + /** + * Percent-decodes {@code value} exactly once, the way the container does for + * a path: {@code %XX} with two ASCII hex digits only, {@code +} left alone. + * + * @return the decoded value, or {@code null} if an escape is malformed, is + * not valid UTF-8, or decodes to {@code %}, {@code ?} or {@code #} + */ + private static String decodeOnce(String value) { + if (value.indexOf('%') == -1) { + return value; + } + StringBuilder decoded = new StringBuilder(value.length()); + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + int i = 0; + while (i < value.length()) { + char c = value.charAt(i); + if (c != '%') { + decoded.append(c); + i++; + continue; + } + // A run of escapes is one byte sequence: a multi-byte character has + // to be decoded as a whole. + bytes.reset(); + while (i < value.length() && value.charAt(i) == '%') { + if (i + 2 >= value.length()) { + return null; + } + int hi = hexDigit(value.charAt(i + 1)); + int lo = hexDigit(value.charAt(i + 2)); + if (hi < 0 || lo < 0) { + return null; + } + int b = (hi << 4) | lo; + if (b == '%' || b == '?' || b == '#') { + // A second encoding layer, or a delimiter the container would + // map literally: never a plain in-app path. + return null; + } + bytes.write(b); + i += 3; + } + try { + decoded.append(StandardCharsets.UTF_8.newDecoder() + .onMalformedInput(CodingErrorAction.REPORT) + .onUnmappableCharacter(CodingErrorAction.REPORT) + .decode(ByteBuffer.wrap(bytes.toByteArray()))); + } catch (CharacterCodingException e) { + // Overlong or truncated sequences: the container would turn them + // into replacement characters, never into a path this code means. + return null; + } + } + return decoded.toString(); + } + + private static int hexDigit(char c) { + if (c >= '0' && c <= '9') { + return c - '0'; + } + if (c >= 'a' && c <= 'f') { + return c - 'a' + 10; + } + if (c >= 'A' && c <= 'F') { + return c - 'A' + 10; + } + return -1; + } +} diff --git a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/services/logout/FSReturnLogoutServlet.java b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/services/logout/FSReturnLogoutServlet.java index d348854052..83b0130a19 100644 --- a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/services/logout/FSReturnLogoutServlet.java +++ b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/services/logout/FSReturnLogoutServlet.java @@ -37,6 +37,7 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import com.sun.identity.federation.common.ForwardPathValidator; import com.sun.identity.federation.common.FSUtils; import com.sun.identity.federation.common.IFSConstants; import com.sun.identity.federation.common.LogUtil; @@ -144,6 +145,15 @@ private void doGetPost(HttpServletRequest request, FSUtils.bundle.getString("aliasNotFound")); return; } + // The alias is appended to the dispatcher path below; a traversal in it + // would forward to an arbitrary resource of the web application. + if (!ForwardPathValidator.isSafeMetaAlias(providerAlias)) { + FSUtils.debug.error("FSReturnLogoutServlet: rejecting metaAlias " + + "with path traversal"); + response.sendError(response.SC_BAD_REQUEST, + FSUtils.bundle.getString("aliasNotFound")); + return; + } Object ssoToken = null; try { diff --git a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/services/logout/FSSingleLogoutServlet.java b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/services/logout/FSSingleLogoutServlet.java index b0969a81ab..c339bf222c 100755 --- a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/services/logout/FSSingleLogoutServlet.java +++ b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/federation/services/logout/FSSingleLogoutServlet.java @@ -24,7 +24,7 @@ * * $Id: FSSingleLogoutServlet.java,v 1.5 2008/12/19 06:50:47 exu Exp $ * - * Portions Copyrighted 2025 3A Systems LLC. + * Portions Copyrighted 2025-2026 3A Systems LLC. */ @@ -37,6 +37,7 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import com.sun.identity.federation.common.ForwardPathValidator; import com.sun.identity.federation.common.FSUtils; import com.sun.identity.federation.common.IFSConstants; import com.sun.identity.federation.services.util.FSServiceUtils; @@ -124,6 +125,15 @@ private void doGetPost(HttpServletRequest request, FSUtils.bundle.getString("aliasNotFound")); return; } + // The alias is appended to the dispatcher path below; a traversal in it + // would forward to an arbitrary resource of the web application. + if (!ForwardPathValidator.isSafeMetaAlias(providerAlias)) { + FSUtils.debug.error("FSSingleLogoutServlet: rejecting metaAlias " + + "with path traversal"); + response.sendError(response.SC_BAD_REQUEST, + FSUtils.bundle.getString("aliasNotFound")); + return; + } request.setAttribute("logoutSource", "local"); StringBuffer processLogout = new StringBuffer(); diff --git a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/saml/common/SAMLUtils.java b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/saml/common/SAMLUtils.java index d4283dd4b7..95b0fea539 100644 --- a/openam-federation/openam-federation-library/src/main/java/com/sun/identity/saml/common/SAMLUtils.java +++ b/openam-federation/openam-federation-library/src/main/java/com/sun/identity/saml/common/SAMLUtils.java @@ -63,6 +63,7 @@ import com.sun.identity.common.TaskRunnable; import com.sun.identity.common.TimerPool; import com.sun.identity.shared.xml.XMLUtils; +import org.apache.commons.lang3.StringEscapeUtils; import com.sun.identity.shared.encode.URLEncDec; import com.sun.identity.shared.encode.Base64; @@ -742,7 +743,10 @@ public static void postToTarget(HttpServletResponse response, PrintWriter out, } out.println("

\n"); } - out.println("
"); + // postYN() only validates host, port and path of the target: the query + // string is caller-supplied, so it must not be able to close the attribute. + out.println(""); if (assertion != null) { it = assertion.iterator(); while (it.hasNext()) { diff --git a/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/common/FSUtilsForwardPathTest.java b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/common/FSUtilsForwardPathTest.java new file mode 100644 index 0000000000..57216f76ab --- /dev/null +++ b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/common/FSUtilsForwardPathTest.java @@ -0,0 +1,101 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.federation.common; + +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +/** + * Drives {@link FSUtils#forwardRequest} with a same-host target, the branch + * that ends in a {@code RequestDispatcher} rather than a redirect. + */ +public class FSUtilsForwardPathTest { + + private static final String BASE = "http://sp.example.com:8080/openam"; + + private String savedDeploymentURI; + private HttpServletRequest request; + private HttpServletResponse response; + + @BeforeMethod + public void setUp() { + // forwardRequest reads the static deployment URI; pinned per test and + // restored whatever the outcome. + savedDeploymentURI = FSUtils.deploymentURI; + FSUtils.deploymentURI = "/openam"; + request = mock(HttpServletRequest.class); + when(request.getServerName()).thenReturn("sp.example.com"); + when(request.getServerPort()).thenReturn(8080); + response = mock(HttpServletResponse.class); + } + + @AfterMethod(alwaysRun = true) + public void restoreDeploymentUri() { + FSUtils.deploymentURI = savedDeploymentURI; + } + + @Test + public void forwardsAnInAppTargetOnTheSameHost() throws Exception { + RequestDispatcher dispatcher = mock(RequestDispatcher.class); + when(request.getRequestDispatcher("/UI/Login?goto=%2Fopenam%2Fconsole")).thenReturn(dispatcher); + + FSUtils.forwardRequest(request, response, BASE + "/UI/Login?goto=%2Fopenam%2Fconsole"); + + verify(dispatcher).forward(request, response); + verify(response, never()).sendError(anyInt()); + } + + @DataProvider + public Object[][] traversingTargets() { + return new Object[][] { + {BASE + "/x/../WEB-INF/web.xml"}, + {BASE + "/WEB-INF/web.xml"}, + // The container decodes the dispatcher path before it normalises it. + {BASE + "/x/%2e%2e/WEB-INF/web.xml"}, + {BASE + "/x/%252e%252e/WEB-INF/web.xml"}, + {BASE + "/x/..%2fWEB-INF/web.xml"}, + {BASE + "/%57EB-INF/web.xml"}, + {BASE + "/x/%zz"}, + // Forms the container collapses to /WEB-INF/web.xml before mapping. + {BASE + "//WEB-INF/web.xml"}, + {BASE + "/./WEB-INF/web.xml"}, + {BASE + "/WEB-INF;x/web.xml"}, + {BASE + "/%2e/WEB-INF/web.xml"}, + {BASE + "/WEB-INF#/x"}, + }; + } + + @Test(dataProvider = "traversingTargets") + public void answers400WithoutDispatchingForATraversingTarget(String url) throws Exception { + FSUtils.forwardRequest(request, response, url); + + verify(response).sendError(HttpServletResponse.SC_BAD_REQUEST); + verify(request, never()).getRequestDispatcher(anyString()); + verify(response, never()).sendRedirect(anyString()); + } +} diff --git a/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/common/ForwardPathValidatorTest.java b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/common/ForwardPathValidatorTest.java new file mode 100644 index 0000000000..fee68b6d5a --- /dev/null +++ b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/common/ForwardPathValidatorTest.java @@ -0,0 +1,151 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.federation.common; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class ForwardPathValidatorTest { + + @DataProvider + public Object[][] safeForwardPaths() { + return new Object[][] { + {"/idpSSOInit.jsp"}, + {"/saml2/jsp/idpSSOInit.jsp?metaAlias=/idp&spEntityID=x"}, + {"/saml2/jsp/idpSSOInit.jsp?metaAlias=%2Fidp&goto=%2F..%2FWEB-INF"}, // query is not a path + {"/console/base/AMAdminFrame"}, + {"/console/my%20page.jsp"}, // one legitimate encoding layer + {"/console/a+b.jsp"}, // '+' is literal in a path + {"/web-info/page.jsp"}, // only the real WEB-INF directory is reserved + {"/a/b..c/d"}, // ".." inside a segment is an ordinary name + {"/console//base/AMAdminFrame"}, // collapses to an ordinary path + {"/./console/base/AMAdminFrame"}, + {"/console/base;jsessionid=1/AMAdminFrame"}, + {"/XUI/#login/"}, // the container drops the fragment: /XUI/ + }; + } + + @DataProvider + public Object[][] unsafeForwardPaths() { + return new Object[][] { + {null}, + {""}, + {"idpSSOInit.jsp"}, // relative + {"/WEB-INF/web.xml"}, + {"/web-inf/classes/x.properties"}, + {"/META-INF/MANIFEST.MF"}, + {"/foo/../WEB-INF/web.xml"}, + {"/foo/../../etc/passwd"}, + {"/foo/..;/WEB-INF/web.xml"}, + {"\\WEB-INF\\web.xml"}, + {"/foo\\..\\WEB-INF"}, + {"/foo/..?x=1"}, + {"/foo\0.jsp"}, + // The container decodes the dispatcher path once more before it + // normalises it, so an encoded traversal must be refused as well. + {"/x/%2e%2e/%2e%2e/WEB-INF/web.xml"}, + {"/x/%2E%2E/WEB-INF/web.xml"}, + {"/x/..%2f..%2fWEB-INF/web.xml"}, + {"/x/%2e%2e%2fWEB-INF/web.xml"}, + {"/x/..%5c..%5cWEB-INF/web.xml"}, + {"/x/..%3b/WEB-INF/web.xml"}, + {"/%57EB-INF/web.xml"}, + {"/%77eb-inf/web.xml"}, + {"/x/%00.jsp"}, + {"/x/%0d%0a.jsp"}, + // A second encoding layer is never a legitimate in-app path, and + // a container that decodes twice would resolve it to a traversal. + {"/x/%252e%252e/%252e%252e/WEB-INF/web.xml"}, + {"/x/%2525.jsp"}, + // A malformed escape makes the container throw; refuse it up front. + {"/x/%zz.jsp"}, + {"/x/%2"}, + {"/x/100%.jsp"}, + // The container collapses "//", "/./" and ";params" before mapping, + // so the reserved-directory check has to see the collapsed path. + {"//WEB-INF/web.xml"}, + {"/./WEB-INF/web.xml"}, + {"/WEB-INF;x/web.xml"}, + {"/;/WEB-INF/web.xml"}, + {"/%2e/WEB-INF/web.xml"}, + {"/WEB-INF/"}, + {"/WEB-INF/./web.xml"}, + // request.getRequestDispatcher() drops a fragment before mapping. + {"/WEB-INF#/x"}, + {"/WEB-INF/web.xml#x"}, + {"/x#/../WEB-INF/web.xml"}, // a ServletContext dispatcher keeps the fragment + // An escape that decodes to a URL delimiter is never a plain in-app path. + {"/WEB-INF%3fx/web.xml"}, + {"/WEB-INF%23/web.xml"}, + {"/x/\u007f.jsp"}, + {"/x/%7f.jsp"}, + {"/x/%c0%ae%c0%ae/WEB-INF/web.xml"}, // overlong UTF-8 is not a path + }; + } + + @Test(dataProvider = "safeForwardPaths") + public void acceptsOrdinaryInAppPaths(String path) { + assertThat(ForwardPathValidator.isSafeForwardPath(path)).as(path).isTrue(); + } + + @Test(dataProvider = "unsafeForwardPaths") + public void rejectsTraversalAndReservedDirectories(String path) { + assertThat(ForwardPathValidator.isSafeForwardPath(path)).as(path).isFalse(); + } + + @DataProvider + public Object[][] safeMetaAliases() { + return new Object[][] { + {"/idp"}, + {"/sp"}, + {"/myrealm/sp"}, + {"/my-realm/my_sp.1"}, + {"/my%20realm/sp"}, // raw request URI form of "/my realm/sp" + }; + } + + @DataProvider + public Object[][] unsafeMetaAliases() { + return new Object[][] { + {null}, + {""}, + {"/../../WEB-INF/web.xml"}, + {"/idp/../../WEB-INF/web.xml"}, + {"\\..\\..\\WEB-INF\\web.xml"}, + {"/idp\0"}, + {"/idp?x=/../WEB-INF"}, + {"/%2e%2e/%2e%2e/WEB-INF/web.xml"}, + {"/idp/..%2f..%2fWEB-INF/web.xml"}, + {"/%252e%252e/%252e%252e/WEB-INF/web.xml"}, + {"/idp%00"}, + {"/idp%zz"}, + {"/idp#/../../WEB-INF/web.xml"}, + {"/idp/%c0%ae%c0%ae/WEB-INF/web.xml"}, + }; + } + + @Test(dataProvider = "safeMetaAliases") + public void acceptsOrdinaryMetaAliases(String metaAlias) { + assertThat(ForwardPathValidator.isSafeMetaAlias(metaAlias)).as(metaAlias).isTrue(); + } + + @Test(dataProvider = "unsafeMetaAliases") + public void rejectsMetaAliasesThatEscapeTheHandlerPath(String metaAlias) { + assertThat(ForwardPathValidator.isSafeMetaAlias(metaAlias)).as(metaAlias).isFalse(); + } +} diff --git a/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/services/logout/FSReturnLogoutServletTest.java b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/services/logout/FSReturnLogoutServletTest.java new file mode 100644 index 0000000000..c1ff15dc21 --- /dev/null +++ b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/services/logout/FSReturnLogoutServletTest.java @@ -0,0 +1,91 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.federation.services.logout; + +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import jakarta.servlet.ServletConfig; +import jakarta.servlet.ServletContext; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +/** + * Drives {@code /ReturnLogout/*} through its second alias source: the servlet is + * prefix-mapped, so with no {@code metaAlias} parameter the alias is cut out of + * the raw, still-encoded {@code getRequestURI()}. + */ +public class FSReturnLogoutServletTest { + + private FSReturnLogoutServlet servlet; + private ServletContext servletContext; + private HttpServletRequest request; + private HttpServletResponse response; + + @BeforeMethod + public void setUp() throws Exception { + servletContext = mock(ServletContext.class); + ServletConfig config = mock(ServletConfig.class); + when(config.getServletContext()).thenReturn(servletContext); + servlet = new FSReturnLogoutServlet(); + servlet.init(config); + request = mock(HttpServletRequest.class); + when(request.getParameter("metaAlias")).thenReturn(null); + response = mock(HttpServletResponse.class); + } + + @Test + public void anOrdinaryAliasInTheRequestUriPassesTheGuard() throws Exception { + when(request.getRequestURI()).thenReturn("/openam/ReturnLogout/metaAlias/idp"); + + servlet.doGet(request, response); + + // No session in a unit test: the servlet fails later, but not on the alias. + verify(response, never()).sendError(eq(HttpServletResponse.SC_BAD_REQUEST), anyString()); + verify(response).sendError(eq(HttpServletResponse.SC_INTERNAL_SERVER_ERROR), anyString()); + } + + @DataProvider + public Object[][] traversingRequestUris() { + return new Object[][] { + {"/openam/ReturnLogout/metaAlias/../../WEB-INF/web.xml"}, + {"/openam/ReturnLogout/metaAlias/idp/..%2f..%2fWEB-INF/web.xml"}, + {"/openam/ReturnLogout/metaAlias/%2e%2e/%2e%2e/WEB-INF/web.xml"}, + {"/openam/ReturnLogout/metaAlias/%252e%252e/%252e%252e/WEB-INF/web.xml"}, + }; + } + + @Test(dataProvider = "traversingRequestUris") + public void answers400WithoutDispatchingForATraversingAliasInTheRequestUri(String uri) throws Exception { + when(request.getRequestURI()).thenReturn(uri); + + servlet.doGet(request, response); + + verify(response).sendError(eq(HttpServletResponse.SC_BAD_REQUEST), anyString()); + verify(response, never()).sendError(eq(HttpServletResponse.SC_INTERNAL_SERVER_ERROR), anyString()); + verify(servletContext, never()).getRequestDispatcher(anyString()); + verify(request, never()).getRequestDispatcher(anyString()); + verify(response, never()).sendError(anyInt()); + } +} diff --git a/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/services/logout/FSSingleLogoutServletTest.java b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/services/logout/FSSingleLogoutServletTest.java new file mode 100644 index 0000000000..45996a8ab9 --- /dev/null +++ b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/federation/services/logout/FSSingleLogoutServletTest.java @@ -0,0 +1,90 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.federation.services.logout; + +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.ServletConfig; +import jakarta.servlet.ServletContext; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +/** + * Drives the unauthenticated {@code /liberty-logout} endpoint end to end: the + * request parameter goes in, the dispatcher path (or the error status) comes out. + */ +public class FSSingleLogoutServletTest { + + private FSSingleLogoutServlet servlet; + private ServletContext servletContext; + private HttpServletRequest request; + private HttpServletResponse response; + + @BeforeMethod + public void setUp() throws Exception { + servletContext = mock(ServletContext.class); + ServletConfig config = mock(ServletConfig.class); + when(config.getServletContext()).thenReturn(servletContext); + servlet = new FSSingleLogoutServlet(); + servlet.init(config); + request = mock(HttpServletRequest.class); + response = mock(HttpServletResponse.class); + } + + @Test + public void forwardsAnOrdinaryAliasToTheProcessLogoutHandler() throws Exception { + when(request.getParameter("metaAlias")).thenReturn("/idp"); + RequestDispatcher dispatcher = mock(RequestDispatcher.class); + when(servletContext.getRequestDispatcher("/ProcessLogout/metaAlias/idp")).thenReturn(dispatcher); + + servlet.doGet(request, response); + + verify(dispatcher).forward(request, response); + verify(response, never()).sendError(anyInt(), anyString()); + } + + @DataProvider + public Object[][] traversingAliases() { + return new Object[][] { + {"/../../WEB-INF/web.xml"}, + // getParameter() has already decoded one layer; the container decodes + // the dispatcher path once more before normalising it. + {"/%2e%2e/%2e%2e/WEB-INF/web.xml"}, + {"/%252e%252e/%252e%252e/WEB-INF/web.xml"}, + {"/idp/..%2f..%2fWEB-INF/web.xml"}, + }; + } + + @Test(dataProvider = "traversingAliases") + public void answers400WithoutDispatchingForATraversingAlias(String metaAlias) throws Exception { + when(request.getParameter("metaAlias")).thenReturn(metaAlias); + + servlet.doGet(request, response); + + verify(response).sendError(eq(HttpServletResponse.SC_BAD_REQUEST), anyString()); + verify(servletContext, never()).getRequestDispatcher(anyString()); + } +} diff --git a/openam-federation/openam-federation-library/src/test/java/com/sun/identity/saml/common/SAMLUtilsTest.java b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/saml/common/SAMLUtilsTest.java new file mode 100644 index 0000000000..ce36c8c430 --- /dev/null +++ b/openam-federation/openam-federation-library/src/test/java/com/sun/identity/saml/common/SAMLUtilsTest.java @@ -0,0 +1,44 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package com.sun.identity.saml.common; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.Collections; + +import org.testng.annotations.Test; + +public class SAMLUtilsTest { + + /** + * postYN() only checks host, port and path of the target, so the query string + * is attacker-influenced and must not be able to close the ACTION attribute. + */ + @Test + public void postToTargetEscapesTargetUrlInFormAction() throws Exception { + String target = "https://sp.example.com/acs?next=\">"); + assertThat(out).contains("ACTION=\"https://sp.example.com/acs?next="><script>"); + } +}