Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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());
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
if (name == null || name.isEmpty() || name.equals(".") || name.equals("..")
|| name.indexOf('/') != -1 || name.indexOf('\\') != -1
|| name.indexOf('\0') != -1) {
debug.error("FilesRepo.constructFile: invalid identity name: " + name);
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());
Expand Down Expand Up @@ -1709,13 +1721,24 @@ 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);
}
}

Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* 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.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.
*/
public final class ForwardPathValidator {

private ForwardPathValidator() {
}

/**
* Whether {@code path} may be forwarded to as-is: absolute, without {@code ..}
* segments (path parameters stripped, as the container does), without
* backslashes or control characters, and not under a reserved directory.
*
* @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);
if (containsTraversal(uri)) {
return false;
}
String lower = uri.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() && !containsTraversal(metaAlias);
}

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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/


Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -742,7 +743,10 @@ public static void postToTarget(HttpServletResponse response, PrintWriter out,
}
out.println("</P>\n");
}
out.println("<FORM METHOD=\"POST\" ACTION=\"" + targeturl + "\">");
// 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("<FORM METHOD=\"POST\" ACTION=\""
+ StringEscapeUtils.escapeHtml4(targeturl) + "\">");
if (assertion != null) {
it = assertion.iterator();
while (it.hasNext()) {
Expand Down
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -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=\"><script>alert(1)</script><a href=\"";
StringWriter html = new StringWriter();

SAMLUtils.postToTarget(null, new PrintWriter(html), Collections.emptyList(), target,
Collections.emptyMap());

String out = html.toString();
assertThat(out).doesNotContain("<script>");
assertThat(out).contains("ACTION=\"https://sp.example.com/acs?next=&quot;&gt;&lt;script&gt;");
}
}
Loading