[
https://issues.apache.org/jira/browse/WW-5826?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gouri Sankar A updated WW-5826:
-------------------------------
Description:
ScopeInterceptor keeps a static IdentityHashMap named locks. lock() and
unlock() modify it inside synchronized (o), where o is the session map.
Dispatcher creates a new SessionMap per request, so concurrent requests do not
share that monitor and the map is modified without mutual exclusion. Under
concurrent use the map's size counter was observed to be corrupted (positive
and negative values). This is hardening; no security impact was demonstrated.
The interceptor is opt-in: it is declared as "scope" in struts-default.xml but
is not part of any bundled interceptor stack, so an application has to
reference it explicitly.
Affected versions: the same code (a static IdentityHashMap locks guarded by
synchronized (o)) is present in the 7.0.0, 7.3.0 and 7.4.0 tags, by source
inspection. The race itself was only reproduced on main. The 6.12.0 tag differs
(lock() synchronizes on locks), so 6.x is not claimed.
Fix: wrap the map with Collections.synchronizedMap, consistent with the
synchronized (locks) in clearLocks().
Tests: ScopeInterceptorTest#testConcurrentLockAndUnlockLeavesNoEntries. It
failed in every run without the fix (25 runs) and passed 20/20 with it. It is
statistical: it can pass without the fix on a fast machine.
Pull request: https://github.com/apache/struts/pull/2082
was:
ScopeInterceptor keeps a static IdentityHashMap named locks. lock() and
unlock() modify it inside synchronized (o), where o is the session map.
Dispatcher creates a new SessionMap per request, so concurrent requests do not
share that monitor and the map is modified without mutual exclusion. Under
concurrent use the map's size counter was observed to be corrupted (positive
and negative values). This is hardening; no security impact was demonstrated.
The interceptor is opt-in (scope and scopedModelDriven stacks, not
defaultStack).
Affected versions: the same code (a static IdentityHashMap locks guarded by
synchronized (o)) is present in the 7.0.0, 7.3.0 and 7.4.0 tags, by source
inspection. The race itself was only reproduced on main. The 6.12.0 tag differs
(lock() synchronizes on locks), so 6.x is not claimed.
Fix: wrap the map with Collections.synchronizedMap, consistent with the
synchronized (locks) in clearLocks().
Tests: ScopeInterceptorTest#testConcurrentLockAndUnlockLeavesNoEntries. It
failed in every run without the fix (25 runs) and passed 20/20 with it. It is
statistical: it can pass without the fix on a fast machine.
> Make ScopeInterceptor lock table thread-safe
> --------------------------------------------
>
> Key: WW-5826
> URL: https://issues.apache.org/jira/browse/WW-5826
> Project: Struts 2
> Issue Type: Improvement
> Components: Core
> Affects Versions: 7.0.0, 7.3.0, 7.4.0
> Reporter: Gouri Sankar A
> Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> ScopeInterceptor keeps a static IdentityHashMap named locks. lock() and
> unlock() modify it inside synchronized (o), where o is the session map.
> Dispatcher creates a new SessionMap per request, so concurrent requests do
> not share that monitor and the map is modified without mutual exclusion.
> Under concurrent use the map's size counter was observed to be corrupted
> (positive and negative values). This is hardening; no security impact was
> demonstrated. The interceptor is opt-in: it is declared as "scope" in
> struts-default.xml but is not part of any bundled interceptor stack, so an
> application has to reference it explicitly.
> Affected versions: the same code (a static IdentityHashMap locks guarded by
> synchronized (o)) is present in the 7.0.0, 7.3.0 and 7.4.0 tags, by source
> inspection. The race itself was only reproduced on main. The 6.12.0 tag
> differs (lock() synchronizes on locks), so 6.x is not claimed.
> Fix: wrap the map with Collections.synchronizedMap, consistent with the
> synchronized (locks) in clearLocks().
> Tests: ScopeInterceptorTest#testConcurrentLockAndUnlockLeavesNoEntries. It
> failed in every run without the fix (25 runs) and passed 20/20 with it. It is
> statistical: it can pass without the fix on a fast machine.
> Pull request: https://github.com/apache/struts/pull/2082
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
