While triaging your project, our bug fixing tool found an issue that generated the following message:
In file: TypeCache.java, method: findOrInsert, there is a synchronized block on a parameter. iCR suggested the block to be synchronized on a private and final field, not on a parameter. To learn more check CWE-412 and CERT Secure coding standard LCK00-J.
Notes from Triage Team
On first look, it looks like a method parameter called monitor of Object type is passed to the method findOrInsert. This doesn't feel right since synchronization should be performed on a fixed or an static object.
The problem with synchronizing on a method parameter is that it's not thread-safe. Imagine two threads calling this method with different values of monitor. Each thread will think it has its own private lock on the object, which leads to unexpected behavior when they try to access the same shared resource.
As a result, we think that synchronization shouldn't be done on a method parameter.
Please let us know whether you think our assumption is wrong.
Sponsorship and Support
This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed – to improve global software supply chain security.
The bug is found by running the Intelligent Code Repair (iCR) tool by OpenRefactory and then manually triaging the results.
(This issue may look like automated, generated by a bot. But actually it isn't)
raphw
commented
1 month ago
Hi,
While triaging your project, our bug fixing tool found an issue that generated the following message:
Notes from Triage Team
On first look, it looks like a method parameter called
monitorofObjecttype is passed to the methodfindOrInsert. This doesn't feel right since synchronization should be performed on a fixed or an static object.The problem with synchronizing on a method parameter is that it's not thread-safe. Imagine two threads calling this method with different values of monitor. Each thread will think it has its own private lock on the object, which leads to unexpected behavior when they try to access the same shared resource.
As a result, we think that synchronization shouldn't be done on a method parameter.
Please let us know whether you think our assumption is wrong.
Sponsorship and Support
This work is done by the security researchers from OpenRefactory and is supported by the Open Source Security Foundation (OpenSSF): Project Alpha-Omega. Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed – to improve global software supply chain security.
The bug is found by running the Intelligent Code Repair (iCR) tool by OpenRefactory and then manually triaging the results.
(This issue may look like automated, generated by a bot. But actually it isn't)