Skip to content

Commit

Permalink
Merge pull request #1583 from fl4via/2.2.x_backport-fixes
Browse files Browse the repository at this point in the history
[UNDERTOW-2264] [UNDERTOW-2374] [UNDERTOW-2375] CVE-2023-1973 Backport fixes/enhancements to 2.2.x branch
  • Loading branch information
fl4via committed Apr 19, 2024
2 parents 02fa346 + c92301d commit c72db34
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 15 deletions.
17 changes: 8 additions & 9 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Bug fixes and documentation improvements are welcome! If you want to contribute,

PRs must be submitted to master branch (soon to be [renamed to main](https://issues.redhat.com/browse/UNDERTOW-2043)) and they should:
- state clearly what they do (see more [here](https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html))
- point to associated Jira (see more on [link par aa sesscao do Jira])
- point to associated Jira (see more [here](#Issues))
- contain a test case, unless existing tests already verify the code added by the PR
- have a license header in all new files, with current year’s number
- pass CI (except for known failures, we are working on fixing those, tracked by [UNDERTOW-1523](https://issues.redhat.com/browse/UNDERTOW-1523))
Expand All @@ -18,9 +18,9 @@ We expect all contributors and users to follow our [Code of Conduct](CODE_OF_CON

# Issues

Undertow uses JIRA to manage issues. All issues can be found [here](https://issues.redhat.com/projects/Undertow/issues).
Undertow uses Jira to manage issues. All issues can be found [here](https://issues.redhat.com/projects/UNDERTOW/issues).

To create a new issue, comment on an existing issue, or assign an issue to yourself, you'll need to first [create a JIRA account](https://issues.redhat.com/).
To create a new issue, comment on an existing issue, or assign an issue to yourself, you'll need to first [create a Jira account](https://issues.redhat.com/).

## Good First Issues

Expand All @@ -31,15 +31,14 @@ Once you have selected an issue you would like to work on, make sure it's not al
## Discussing your Planned Changes

If you want feedback, you can discuss your planned changes in any of the following ways:
* add comments to the issue ticket at [Undertow Jira](https://issues.jboss.org/browse/UNDERTOW)
* add comments to the issue ticket at [Undertow Jira](https://issues.redhat.com/browse/UNDERTOW)
* Undertow Dev Google Group
* Undertow [Zulip chat](https://wildfly.zulipchat.com/#narrow/stream/174183-undertow "#undertow").
* or simply create a draft PR and poing in the PR description that you would like feedback on the proposal before getting to the
* or simply create a draft PR and point out in the PR description that you would like feedback on the proposal before getting to the
final solution


PR Review Process
--------------------------------------------
# PR Review Process

PR reviewers will take into account the following aspects when reviewing your PR:
- correctness: the code must be correct
Expand All @@ -61,9 +60,9 @@ Besides the classifications labels, a series of labels are going to be added to
- **waiting peer review** PR has been reviewed but is waiting on a second review before being merged (as the changes affects core classes or adds a new feature)
- **next release** PR is in the payload of the next release based on master branch
- **maintenance branch** this tag is used for PRs submitted to maintenance branches only, and is included here just for completeness. Maintainers will take care of backporting
submittted fixes to the maintenance branches when needed
submitted fixes to the maintenance branches when needed

#GitHub Quickstart
# GitHub Quickstart

If this is your first time contributing to a GitHub project, you can follow the next steps to get up to speed when contributing to
Undertow. Regardless of your level of experience, though, we kindly ask you that PRs are always rebased before being submitted or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,22 @@
public class FormAuthenticationMechanism implements AuthenticationMechanism {

public static final String LOCATION_ATTRIBUTE = FormAuthenticationMechanism.class.getName() + ".LOCATION";

public static final String DEFAULT_POST_LOCATION = "/j_security_check";

protected static final String ORIGINAL_SESSION_TIMEOUT = "io.undertow.servlet.form.auth.orig.session.timeout";;
private final String name;
private final String loginPage;
private final String errorPage;
private final String postLocation;
private final FormParserFactory formParserFactory;
private final IdentityManager identityManager;

/**
* If the authentication process creates a session, this is the maximum session timeout (in seconds) during the
* authentication process. Once authentication is complete, the default session timeout will apply. Sessions that
* exist before the authentication process starts will retain their original session timeout throughout.
*/
protected final int authenticationSessionTimeout = 120;

public FormAuthenticationMechanism(final String name, final String loginPage, final String errorPage) {
this(FormParserFactory.builder().build(), name, loginPage, errorPage);
}
Expand Down Expand Up @@ -166,6 +172,10 @@ public AuthenticationMechanismOutcome runFormAuth(final HttpServerExchange excha
protected void handleRedirectBack(final HttpServerExchange exchange) {
final Session session = Sessions.getSession(exchange);
if (session != null) {
final Integer originalSessionTimeout = (Integer) session.removeAttribute(ORIGINAL_SESSION_TIMEOUT);
if (originalSessionTimeout != null) {
session.setMaxInactiveInterval(originalSessionTimeout);
}
final String location = (String) session.removeAttribute(LOCATION_ATTRIBUTE);
if(location != null) {
exchange.addDefaultResponseListener(new DefaultResponseListener() {
Expand Down Expand Up @@ -208,7 +218,19 @@ public ChallengeResult sendChallenge(final HttpServerExchange exchange, final Se
}

protected void storeInitialLocation(final HttpServerExchange exchange) {
Session session = Sessions.getOrCreateSession(exchange);
Session session = Sessions.getSession(exchange);
boolean newSession = false;
if (session == null) {
session = Sessions.getOrCreateSession(exchange);
newSession = true;
}
if (newSession) {
int originalMaxInactiveInterval = session.getMaxInactiveInterval();
if (originalMaxInactiveInterval > authenticationSessionTimeout) {
session.setAttribute(ORIGINAL_SESSION_TIMEOUT, session.getMaxInactiveInterval());
session.setMaxInactiveInterval(authenticationSessionTimeout);
}
}
session.setAttribute(LOCATION_ATTRIBUTE, RedirectBuilder.redirect(exchange, exchange.getRelativePath()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,9 @@ private boolean checkRequestHeaders(HeaderMap headers) {
}

// verify content of request pseudo-headers. Each header should only have a single value.
if (headers.contains(PATH)) {
if (headers.contains(PATH) && !allowUnescapedCharactersInUrl) {
for (byte b: headers.get(PATH).getFirst().getBytes(ISO_8859_1)) {
if (!allowUnescapedCharactersInUrl && !HttpRequestParser.isTargetCharacterAllowed((char)b)){
if (!HttpRequestParser.isTargetCharacterAllowed((char)b)){
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import io.undertow.servlet.handlers.ServletRequestContext;
import io.undertow.servlet.spec.HttpSessionImpl;
import io.undertow.servlet.util.SavedRequest;
import io.undertow.servlet.spec.ServletContextImpl;
import io.undertow.util.Headers;
import io.undertow.util.RedirectBuilder;

Expand Down Expand Up @@ -195,13 +196,26 @@ protected void storeInitialLocation(final HttpServerExchange exchange, byte[] by
return;
}
final ServletRequestContext servletRequestContext = exchange.getAttachment(ServletRequestContext.ATTACHMENT_KEY);
HttpSessionImpl httpSession = servletRequestContext.getCurrentServletContext().getSession(exchange, true);
final ServletContextImpl servletContextImpl = servletRequestContext.getCurrentServletContext();
HttpSessionImpl httpSession = servletContextImpl.getSession(exchange, false);
boolean newSession = false;
if (httpSession == null) {
httpSession = servletContextImpl.getSession(exchange, true);
newSession = true;
}
Session session;
if (System.getSecurityManager() == null) {
session = httpSession.getSession();
} else {
session = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(httpSession));
}
if (newSession) {
int originalMaxInactiveInterval = session.getMaxInactiveInterval();
if (originalMaxInactiveInterval > authenticationSessionTimeout) {
session.setAttribute(ORIGINAL_SESSION_TIMEOUT, session.getMaxInactiveInterval());
session.setMaxInactiveInterval(authenticationSessionTimeout);
}
}
SessionManager manager = session.getSessionManager();
if (seenSessionManagers.add(manager)) {
manager.registerSessionListener(LISTENER);
Expand All @@ -226,6 +240,10 @@ protected void handleRedirectBack(final HttpServerExchange exchange) {
} else {
session = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(httpSession));
}
Integer originalSessionTimeout = (Integer) session.removeAttribute(ORIGINAL_SESSION_TIMEOUT);
if (originalSessionTimeout != null) {
session.setMaxInactiveInterval(originalSessionTimeout);
}
String path = (String) session.getAttribute(SESSION_KEY);
if ((path == null || overrideInitial) && defaultPage != null) {
path = defaultPage;
Expand Down

0 comments on commit c72db34

Please sign in to comment.