[
https://issues.apache.org/jira/browse/CXF-9221?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Guanping Zhang updated CXF-9221:
--------------------------------
Description:
In the JCache-based OAuth data providers ({{{}JCacheCodeDataProvider{}}} and
{{{}JCacheOAuthDataProvider{}}}), the overridden {{isExpired()}} methods
contain a logic inversion that prevents expired grants and tokens from ever
being reported as expired by the cache eviction path.
The current implementation computes: {{return System.currentTimeMillis() <
(grant.getIssuedAt() + grant.getExpiresIn());}}
There are two issues here:
# Unit Mismatch: {{System.currentTimeMillis()}} returns milliseconds, while
{{getIssuedAt()}} and {{getExpiresIn()}} are in seconds. The millisecond value
(~10¹²) will almost always be greater than the seconds sum (~10⁹), making the
comparison inherently flawed.
# Polarity Inversion: The logical polarity is inverted compared to the correct
implementation in {{{}OAuthUtils.isExpired(){}}}.
As a result, {{isExpired()}} always returns {{false}} for the JCache eviction
checks. While the actual security boundaries (token introspection, refresh
validation) correctly use {{{}OAuthUtils.isExpired(){}}}, this defect causes
expired entries to accumulate indefinitely in the JCache, leading to unbounded
cache growth and potential memory pressure over time.
was:
In the JCache-based OAuth data providers ({{{}JCacheCodeDataProvider{}}} and
{{{}JCacheOAuthDataProvider{}}}), the overridden {{isExpired()}} methods
contain a logic inversion that prevents expired grants and tokens from ever
being reported as expired by the cache eviction path.
The current implementation computes: {{return System.currentTimeMillis() <
(grant.getIssuedAt() + grant.getExpiresIn());}}
There are two issues here: # Unit Mismatch: {{System.currentTimeMillis()}}
returns milliseconds, while {{getIssuedAt()}} and {{getExpiresIn()}} are in
seconds. The millisecond value (~10¹²) will almost always be greater than the
seconds sum (~10⁹), making the comparison inherently flawed.
# Polarity Inversion: The logical polarity is inverted compared to the correct
implementation in {{{}OAuthUtils.isExpired(){}}}.
As a result, {{isExpired()}} always returns {{false}} for the JCache eviction
checks. While the actual security boundaries (token introspection, refresh
validation) correctly use {{{}OAuthUtils.isExpired(){}}}, this defect causes
expired entries to accumulate indefinitely in the JCache, leading to unbounded
cache growth and potential memory pressure over time.
> JCache providers use inverted isExpired() logic causing expired tokens/codes
> to never be evicted
> ------------------------------------------------------------------------------------------------
>
> Key: CXF-9221
> URL: https://issues.apache.org/jira/browse/CXF-9221
> Project: CXF
> Issue Type: Bug
> Components: JAX-RS, JAX-RS Security
> Affects Versions: 4.2.2
> Environment: Apache CXF 3.5.7, Java 11+, JCache (JSR-107)
> Reporter: Guanping Zhang
> Priority: Minor
>
> In the JCache-based OAuth data providers ({{{}JCacheCodeDataProvider{}}} and
> {{{}JCacheOAuthDataProvider{}}}), the overridden {{isExpired()}} methods
> contain a logic inversion that prevents expired grants and tokens from ever
> being reported as expired by the cache eviction path.
>
> The current implementation computes: {{return System.currentTimeMillis() <
> (grant.getIssuedAt() + grant.getExpiresIn());}}
> There are two issues here:
> # Unit Mismatch: {{System.currentTimeMillis()}} returns milliseconds, while
> {{getIssuedAt()}} and {{getExpiresIn()}} are in seconds. The millisecond
> value (~10¹²) will almost always be greater than the seconds sum (~10⁹),
> making the comparison inherently flawed.
> # Polarity Inversion: The logical polarity is inverted compared to the
> correct implementation in {{{}OAuthUtils.isExpired(){}}}.
> As a result, {{isExpired()}} always returns {{false}} for the JCache eviction
> checks. While the actual security boundaries (token introspection, refresh
> validation) correctly use {{{}OAuthUtils.isExpired(){}}}, this defect causes
> expired entries to accumulate indefinitely in the JCache, leading to
> unbounded cache growth and potential memory pressure over time.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
