Affects: all up to 5.3.1-SNAPSHOT
I found some suspicious pattern of the Java code which can lead to concurrent race condition and memory leak. I confirm problematic software pattern in other software, outside of Spring. In Spring-Framework source code I found the same concurrent pattern in places e.g. CacheAspectSupport, AbstractAdvisingBeanPostProcessor, AdvisedSupport, AsyncExecutionInterceptor etc.
Also in other place of Spring-Framework source code it's correctly implemented.
Also I'd like to mention that I'm application developer not framework developer and I don't know internal Spring implementation. I known sounds unlikely. I hope I'm wrong.
I will provide more details via e-mail security@pivotal.io
Kindly regards
Comment From: jhoeller
We don't usually consider race conditions and internal memory leaks as security-sensitive, so feel free to raise them here directly. Of course, if you have can reproduce an externally induced memory leak leading to a potential DoS attack, it's better off as a security report. However, your report above rather sounds like feedback on an implementation pattern based on a code review...
Comment From: jhoeller
If by any chance you mean non-synchronized get/put operations against a ConcurrentHashMap, we tend to use those intentionally, as a trade-off where potential double computation or even storage of two equal values doesn't really matter. We could replace those with computeIfAbsent but that would imply some segment locking at least, a different trade-off. We already use computeIfAbsent where a unique value instance is essential, and sometimes also where the value can be very quickly computed (without any callbacks to potentially expensive SPI implementations). Using it in additional places such as the ones mentioned above sounds like a potential enhancement that we may discuss of course, but not really a security-relevant ticket (except for the case where there is a reproducible unbounded memory leak that can be triggered by externally clients).
Comment From: drnow4u
Hi Juergen,
Thanks for verbose explanation. That kind of issues are difficult to reproduce and code review is probably one of the best way to find it. So, You right my feedback based on a code review.
As discussed it should be possible to get rid of double lock in similar place to AspectJProxyFactory. Always doing change related to thread synchronization is risky. No pushing.
Could you change status of ticket to enhancement as you proposed.
Kindly regards, Marcin
Comment From: bclozel
I think this should have been closed with #30780.