8000 Potential thread safety Issue with lazy init of transformerFactory at TransformerUtil. SAST · Issue #40030 · keycloak/keycloak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
Potential thread safety Issue with lazy init of transformerFactory at TransformerUtil. SAST #40030
Closed
@Anchels

Description

@Anchels

Description

This method contains an unsynchronized lazy initialization of a static field. After the field is set, the object stored into that location is further updated or accessed. The setting of the field is visible to other threads as soon as it is set. If the further accesses in the method that set the field serve to initialize the object, then you have a very serious multithreading bug, unless something else prevents any other thread from accessing the stored object until it is fully initialized.

Even if you feel confident that the method is never called by multiple threads, it might be better to not set the static field until the value you are setting it to is fully populated/initialized.

See CWE-543: Use of Singleton Pattern Without Synchronization in a Multithreaded Context

Discussion

No response

Motivation

See Description

Details

Incorrect lazy initialization and update of static field transformerFactory in method getTransformerFactory() :

private static TransformerFactory transformerFactory;

77CE
/**
* <p>Creates a {@link TransformerFactory}. The returned instance is cached and shared between different
* threads.</p>
*
* @return
*
* @throws TransformerFactoryConfigurationError
*/
public static TransformerFactory getTransformerFactory() throws TransformerFactoryConfigurationError {
if (transformerFactory == null) {
boolean tccl_jaxp = SystemPropertiesUtil.getSystemProperty(GeneralConstants.TCCL_JAXP, "false")
.equalsIgnoreCase("true");
ClassLoader prevTCCL = SecurityActions.getTCCL();
try {
if (tccl_jaxp) {
SecurityActions.setTCCL(TransformerUtil.class.getClassLoader());
}
transformerFactory = TransformerFactory.newInstance();
try {
transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
} catch (TransformerConfigurationException ignored) {
// some platforms don't support this. For example our testsuite pulls Selenium which requires Xalan 2.7.1
logger.warn("XML External Entity switches are not supported. You may get XML injection vulnerabilities.");
}
try {
transformerFactory.setAttribute(FixXMLConstants.ACCESS_EXTERNAL_DTD, "");
transformerFactory.setAttribute(FixXMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
} catch (Exception ignored) {
// some platforms don't support this. For example our testsuite pulls Selenium which requires Xalan 2.7.1
logger.warn("XML External Entity switches are not supported. You may get XML injection vulnerabilities.");
}
} finally {
if (tccl_jaxp) {
SecurityActions.setTCCL(prevTCCL);
}
}
}
return transformerFactory;
}

Problem: The current implementation is not thread-safe. If multiple threads call getTransformerFactory() simultaneously, they may end up creating multiple instances of transformerFactory.

Proposed solution: Consider using "Double-Checked-Locking" pattern for thread-safe lazy initialization.


Found by Linux Verification Center with SVACE

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      0