fix(security): address the three actionable CVEs from the full dependency pass - #57
Draft
kathrynalpert wants to merge 5 commits into
Draft
fix(security): address the three actionable CVEs from the full dependency pass#57kathrynalpert wants to merge 5 commits into
kathrynalpert wants to merge 5 commits into
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This was referenced Sep 4, 2026
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Three follow-ups from a full reachability pass over all 28 Spring-ecosystem jars in
WEB-INF/lib. Of 43 CVEs across those jars, 35 are not applicable, two were fixed in #53, and these are the remaining ones that can be addressed without waiting for the Spring Framework 6 migration.Grouped into one PR because they share a cause and a review context. Each is a separate commit and can be reviewed or dropped independently.
fix(security)headersSecurityConfig— no upgrade availablefix(security)passwordfix(deps)spring-ldapRisks and impact
Security, authentication, and authorization
All three reduce exposure and grant nothing. No roles, permissions or authorization logic change.
CVE-2026-22732 (Critical). When an application configures response headers through Spring Security and
HeaderWriterFilter.shouldWriteHeadersEagerlyis left at its default offalse, those headers can go unwritten on responses that commit early. XNAT meets the precondition —SecurityConfigsets frame options, cache control, a content security policy and a referrer policy — so requests can be served without protections the site believes are in place. No public 5.7.x release has the fix (5.7.22 is commercial), so this applies the vendor's documented workaround via anObjectPostProcessor.The vendor notes a behavioural trade-off: with eager writing, application-specific headers override individual Spring Security headers rather than suppressing them wholesale. Checked, and the two sets are disjoint, so nothing changes hands. Spring Security writes
X-Frame-Options,Content-Security-Policy,Referrer-Policy,X-Content-Type-OptionsandX-XSS-Protection, and XNAT sets none of them. XNAT setsCache-Control(26 sites),PragmaandExpires— and Spring Security'scacheControl()and HSTS writers are both.disable()d inSecurityConfig. So the headers XNAT controls are exactly the ones Spring Security was told not to write.That grep covered XNAT core only. A plugin setting its own CSP or frame options would see its value win after this change — arguably more correct, but a change.
CVE-2025-22228 (High). bcrypt hashes only the first 72 bytes of a password, and this CVE means
matches()ignores everything past that on comparison too — any two passwords sharing a 72-byte prefix both authenticate. XNAT is exposed:PasswordEncoderFactories.createDelegatingPasswordEncoder()makes bcrypt the active encoder, it is wired intoXnatDatabaseAuthenticationProvider, and no maximum length was enforced —primary_passwordisxs:stringwith nomaxLength, the defaultpasswordComplexityis^.*$, and no password input carries amaxlength.CVE-2024-38829 (Medium). The same locale-dependent case-conversion defect as CVE-2024-38827, in Spring LDAP query handling. 2.4.4 is the vendor's fix for the 2.4.x branch and is on Central.
Input data
N/A
Upgrade and compatibility
No schema, data model, REST API or signature changes.
One user-visible change: a password over 72 bytes can no longer be set.
Testing
Unit tests
Added
RegExpValidatorTest(4 tests): 72 bytes accepted, 73 rejected, 25 three-byte characters rejected, ordinary passphrase unaffected. Two of them fail without the change. Suites:xnat-web423 andxdat167, 0 failures.REST integration tests
None added.
Manual verification
Content-Security-Policy,X-Frame-OptionsandReferrer-Policyare present across response types — a normal page, an error page, a static resource, a streamed download — and compare against the same set before the change.