From 46e25428e9fc52b08a8977588274037d1f165656 Mon Sep 17 00:00:00 2001 From: Kate Alpert <313617+kathrynalpert@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:19:32 -0600 Subject: [PATCH 1/5] fix(security): write security headers eagerly (CVE-2026-22732) CVE-2026-22732 (Critical): when an application configures HTTP response headers through Spring Security and HeaderWriterFilter's shouldWriteHeadersEagerly is left at its default of false, those headers can go unwritten on responses that commit early. XNAT meets the precondition -- SecurityConfig configures frame options, cache control, a content security policy and a referrer policy -- so requests can be served without the protections the site believes are in place. No public Spring Security 5.7.x release carries the fix (5.7.22 is commercial), so this applies the vendor's documented workaround, setting shouldWriteHeadersEagerly to true through an ObjectPostProcessor. Note the behavioural trade-off the vendor calls out: with eager writing, application-specific headers override individual Spring Security headers rather than suppressing them wholesale. Co-Authored-By: Claude Opus 5 --- .../nrg/xnat/initialization/SecurityConfig.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/xnat-web/src/main/java/org/nrg/xnat/initialization/SecurityConfig.java b/xnat-web/src/main/java/org/nrg/xnat/initialization/SecurityConfig.java index 5702c44c9..7435050da 100644 --- a/xnat-web/src/main/java/org/nrg/xnat/initialization/SecurityConfig.java +++ b/xnat-web/src/main/java/org/nrg/xnat/initialization/SecurityConfig.java @@ -42,6 +42,7 @@ import org.springframework.security.authentication.AuthenticationEventPublisher; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.authentication.AuthenticationProvider; +import org.springframework.security.config.annotation.ObjectPostProcessor; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; @@ -68,6 +69,7 @@ import org.springframework.security.web.authentication.session.RegisterSessionAuthenticationStrategy; import org.springframework.security.web.authentication.session.SessionFixationProtectionStrategy; import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; +import org.springframework.security.web.header.HeaderWriterFilter; import org.springframework.security.web.header.writers.ReferrerPolicyHeaderWriter.ReferrerPolicy; import org.springframework.security.web.session.SimpleRedirectSessionInformationExpiredStrategy; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; @@ -307,6 +309,18 @@ protected void configure(final HttpSecurity http) throws Exception { .sessionRegistry(sessionRegistry()) .expiredSessionStrategy(new SimpleRedirectSessionInformationExpiredStrategy("/app/template/Login.vm", redirectStrategy(_preferences, detector))); + // CVE-2026-22732: with shouldWriteHeadersEagerly left at its default of false, the headers configured + // below can go unwritten on responses that commit early, leaving requests without the content security + // policy, frame options and referrer policy. No public Spring Security 5.7.x release carries the fix, so + // apply the vendor's documented workaround. + http.headers().addObjectPostProcessor(new ObjectPostProcessor() { + @Override + public O postProcess(final O filter) { + filter.setShouldWriteHeadersEagerly(true); + return filter; + } + }); + http.headers().frameOptions().sameOrigin().cacheControl().disable().contentSecurityPolicy(CONTENT_SECURITY_POLICY) .and().referrerPolicy(ReferrerPolicy.STRICT_ORIGIN_WHEN_CROSS_ORIGIN) .and().httpStrictTransportSecurity().disable() From df9c4bb703a2671bc45a908f4095e4678dbdaf8e Mon Sep 17 00:00:00 2001 From: Kate Alpert <313617+kathrynalpert@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:11:06 -0600 Subject: [PATCH 2/5] fix(deps): bump spring-ldap to 2.4.4 CVE-2024-38829 (Medium) affects Spring LDAP 2.4.3 and earlier: the same class of locale-dependent String.toLowerCase()/toUpperCase() defect as CVE-2024-38827, here causing unintended data queries. 2.4.4 is the vendor's fix for the 2.4.x branch and is published on Maven Central, so unlike the rest of this line it needs no framework migration. This does not clear CVE-2026-41720 (authentication bypass with an empty password), which the vendor lists as affecting 2.4.4 and earlier; its fix is 2.4.5 and is not published publicly. That is tracked separately. Co-Authored-By: Claude Opus 5 --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index adc6344c2..1c14f2681 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -143,7 +143,7 @@ snakeyaml = "1.33" spotbugs = "4.7.3.4" spring-framework = "5.3.39" spring-jwt = "1.1.1.RELEASE" -spring-ldap = "2.4.2" +spring-ldap = "2.4.4" spring-oauth2 = "2.5.2.RELEASE" spring-security = "5.7.14" spring-test-junit5 = "1.5.0" From 992009343f40e45ec85c84078a8b08e3b7bcfab0 Mon Sep 17 00:00:00 2001 From: Kate Alpert <313617+kathrynalpert@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:14:13 -0600 Subject: [PATCH 3/5] fix(security): reject passwords longer than 72 bytes (CVE-2025-22228) bcrypt only hashes the first 72 bytes of a password. CVE-2025-22228 (High) is that BCryptPasswordEncoder.matches() ignores everything past that on comparison too, so any two passwords sharing a 72-byte prefix both authenticate. XNAT is exposed: PasswordEncoderFactories.createDelegatingPasswordEncoder() makes bcrypt the active encoder, SecurityConfig wires it into XnatDatabaseAuthenticationProvider, and no maximum password length was enforced anywhere. A user who set a password longer than 72 bytes had silently capped entropy, and anyone knowing the first 72 bytes could log in as them. No public Spring Security release on the 5.7.x line carries the upstream fix (5.7.16 is commercial), and the lowest public fixed version is 6.3.8, which needs Spring Framework 6. So this removes the precondition in XNAT instead of waiting for the migration: RegExpValidator now rejects passwords over 72 UTF-8 bytes. Rejecting rather than truncating is deliberate. Truncating would hand a user credentials whose tail silently does not count. The limit is measured in bytes, not characters, because that is what bcrypt consumes; 25 three-byte characters is 75 bytes and is refused even though it is well under 72 characters. Co-Authored-By: Claude Opus 5 --- .../security/validators/RegExpValidator.java | 14 ++++++ .../validators/RegExpValidatorTest.java | 49 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 xdat/src/test/java/org/nrg/xdat/security/validators/RegExpValidatorTest.java diff --git a/xdat/src/main/java/org/nrg/xdat/security/validators/RegExpValidator.java b/xdat/src/main/java/org/nrg/xdat/security/validators/RegExpValidator.java index e849b6f81..92bed4f3a 100644 --- a/xdat/src/main/java/org/nrg/xdat/security/validators/RegExpValidator.java +++ b/xdat/src/main/java/org/nrg/xdat/security/validators/RegExpValidator.java @@ -15,6 +15,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; +import java.nio.charset.StandardCharsets; import java.util.regex.Pattern; @Component @@ -34,6 +35,13 @@ public RegExpValidator(final SiteConfigPreferences preferences) { @Override public String isValid(final String password, final UserI user) { + // bcrypt only hashes the first 72 bytes of a password, and CVE-2025-22228 means anything beyond that is + // ignored on comparison as well: two passwords sharing a 72-byte prefix both authenticate. Reject rather + // than silently truncate, so a user is never given credentials whose tail does not count. + if (password != null && password.getBytes(StandardCharsets.UTF_8).length > MAX_PASSWORD_BYTES) { + return "Password must be " + MAX_PASSWORD_BYTES + " characters or fewer."; + } + final String regexp = getPasswordComplexity(); return StringUtils.isBlank(regexp) || Pattern.matches(regexp, password) ? "" @@ -48,5 +56,11 @@ private String getPasswordComplexityMessage() { return _preferences != null ? _preferences.getPasswordComplexityMessage() : "Password is not sufficiently complex."; } + /** + * The maximum number of bytes bcrypt incorporates into a hash. Passwords longer than this are rejected: see + * CVE-2025-22228. + */ + private static final int MAX_PASSWORD_BYTES = 72; + private final SiteConfigPreferences _preferences; } diff --git a/xdat/src/test/java/org/nrg/xdat/security/validators/RegExpValidatorTest.java b/xdat/src/test/java/org/nrg/xdat/security/validators/RegExpValidatorTest.java new file mode 100644 index 000000000..4c7f8225d --- /dev/null +++ b/xdat/src/test/java/org/nrg/xdat/security/validators/RegExpValidatorTest.java @@ -0,0 +1,49 @@ +/* + * core: org.nrg.xdat.security.validators.RegExpValidatorTest + * XNAT http://www.xnat.org + * Copyright (c) 2005-2026, Washington University School of Medicine and Howard Hughes Medical Institute + * All Rights Reserved + * + * Released under the Simplified BSD. + */ + +package org.nrg.xdat.security.validators; + +import org.junit.Test; + +import java.nio.charset.StandardCharsets; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.apache.commons.lang3.StringUtils.repeat; + +/** + * Covers the password length ceiling added for CVE-2025-22228. The package-protected constructor gives the + * "panic mode" defaults, so the complexity pattern is {@code ^.*$} and length is the only thing under test. + */ +public class RegExpValidatorTest { + @Test + public void passwordAtTheByteLimitIsAccepted() { + assertThat(validator.isValid(repeat("a", 72), null)).isEmpty(); + } + + @Test + public void passwordOneByteOverTheLimitIsRejected() { + assertThat(validator.isValid(repeat("a", 73), null)).contains("72 characters or fewer"); + } + + @Test + public void multiByteCharactersCountAsBytesNotCharacters() { + // 25 three-byte characters is 75 bytes, so this is rejected despite being well under 72 characters. + final String password = repeat("δΈ­", 25); + assertThat(password.length()).isLessThan(72); + assertThat(password.getBytes(StandardCharsets.UTF_8).length).isGreaterThan(72); + assertThat(validator.isValid(password, null)).contains("72 characters or fewer"); + } + + @Test + public void ordinaryPasswordIsUnaffected() { + assertThat(validator.isValid("correct horse battery staple", null)).isEmpty(); + } + + private final RegExpValidator validator = new RegExpValidator(); +} From 48300ce5e57f63f3a19a2d1a5bb029a4636f1e70 Mon Sep 17 00:00:00 2001 From: Kate Alpert <313617+kathrynalpert@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:39:34 -0600 Subject: [PATCH 4/5] fix(xapi): return 400, not 500, when user creation rejects a password PasswordComplexityException already carries @ResponseStatus(BAD_REQUEST), but createUser's blanket catch rewrapped every exception as UserInitException, which carries INTERNAL_SERVER_ERROR. So a rejected password surfaced as a 500 with the reason buried in the body, and a client could not tell bad input from a server fault. updateUser already special-cases this exception for exactly this reason; createUser was simply inconsistent. This mirrors it, throwing DataFormatException, which is already declared on the method and carries BAD_REQUEST, so no signature changes. Verified on a deployed instance: POST /xapi/users with a 94-character password previously returned 500, and now returns 400 with "Password must be 72 characters or fewer." The path was near-unreachable before the 72-byte limit in this branch, since the default passwordComplexity of ^.*$ never rejects anything. Co-Authored-By: Claude Opus 5 --- xnat-web/src/main/java/org/nrg/xapi/rest/users/UsersApi.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/xnat-web/src/main/java/org/nrg/xapi/rest/users/UsersApi.java b/xnat-web/src/main/java/org/nrg/xapi/rest/users/UsersApi.java index a2fb892ff..5ee52204d 100644 --- a/xnat-web/src/main/java/org/nrg/xapi/rest/users/UsersApi.java +++ b/xnat-web/src/main/java/org/nrg/xapi/rest/users/UsersApi.java @@ -342,6 +342,11 @@ public User createUser(@RequestBody final User model) throws DataFormatException } } return _factory.getUser(user); + } catch (PasswordComplexityException e) { + // Mirrors updateUser(): a rejected password is bad input, so it must not be rewrapped as + // UserInitException, which carries INTERNAL_SERVER_ERROR. DataFormatException is already declared + // here and carries BAD_REQUEST. + throw new DataFormatException(e.getMessage(), e); } catch (Exception e) { throw new UserInitException("Error occurred creating user " + user.getLogin() + " Cause: " + e.getMessage(), e); } From 82542472c96d1325d260bba0296ed3c7c500cd38 Mon Sep 17 00:00:00 2001 From: Kate Alpert <313617+kathrynalpert@users.noreply.github.com> Date: Fri, 4 Sep 2026 12:00:47 -0600 Subject: [PATCH 5/5] fix(xapi): honour XapiException's status instead of always returning 500 XapiRestControllerAdvice derived a response status solely from the @ResponseStatus annotation on the exception class: final ResponseStatus annotation = findAnnotation(throwable.getClass(), ResponseStatus.class); return annotation != null ? annotation.value() : DEFAULT_ERROR_STATUS; XapiException carries its intended status as a constructor argument and a getStatus() field, not an annotation, and nothing ever read that field. So every `throw new XapiException(HttpStatus.BAD_REQUEST, ...)` in the codebase returned 500 instead. Five of the six call sites intend BAD_REQUEST, including the password-complexity branch in UsersApi.updateUser, which looked correct and never was. Also note getExceptionResponseEntity's comment claims an explicitly passed status takes precedence, while the code prefers the exception-derived one whenever a throwable is present. That is left alone here; the annotated exceptions all agree with the status their handlers pass, so it is currently harmless. The fix is additive: the annotation still wins where present, so every exception that works today is unchanged, and an un-annotated XapiException now yields its own status rather than 500. Verified on a deployed instance: PUT /xapi/users/{username} with a 94-character password returned 500 before and 400 after. Co-Authored-By: Claude Opus 5 --- .../nrg/xapi/rest/XapiRestControllerAdvice.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/xnat-web/src/main/java/org/nrg/xapi/rest/XapiRestControllerAdvice.java b/xnat-web/src/main/java/org/nrg/xapi/rest/XapiRestControllerAdvice.java index e5bb14174..7838c111f 100644 --- a/xnat-web/src/main/java/org/nrg/xapi/rest/XapiRestControllerAdvice.java +++ b/xnat-web/src/main/java/org/nrg/xapi/rest/XapiRestControllerAdvice.java @@ -24,6 +24,7 @@ import org.nrg.xapi.exceptions.NoContentException; import org.nrg.xapi.exceptions.NotAuthenticatedException; import org.nrg.xapi.exceptions.NotFoundException; +import org.nrg.xapi.exceptions.XapiException; import org.nrg.xapi.exceptions.ResourceAlreadyExistsException; import org.nrg.xdat.XDAT; import org.nrg.xdat.preferences.SiteConfigPreferences; @@ -213,7 +214,18 @@ private ResponseEntity getExceptionResponseEntity(@Nonnull final HttpServletR private HttpStatus getExceptionResponseStatus(final Throwable throwable) { final ResponseStatus annotation = AnnotationUtils.findAnnotation(throwable.getClass(), ResponseStatus.class); - return annotation != null ? annotation.value() : DEFAULT_ERROR_STATUS; + if (annotation != null) { + return annotation.value(); + } + // XapiException carries its intended status as a field rather than an annotation. Without this, every + // `throw new XapiException(BAD_REQUEST, ...)` silently became a 500. + if (throwable instanceof XapiException) { + final HttpStatus status = ((XapiException) throwable).getStatus(); + if (status != null) { + return status; + } + } + return DEFAULT_ERROR_STATUS; } private static final HttpStatus DEFAULT_ERROR_STATUS = INTERNAL_SERVER_ERROR;