No, but it should absolutely be regarded as vulnerable (the problem with being a “platform” in these circumstances is even if there were no way to exploit anything in core, any module might expose the vulnerability).
Hi @ibacher Can I request you to make a release on Core 2.1.1(WAR only) with the latest log4j 2.17.1 version. Core 2.1.1 is being used for 0.92 and 0.93 versions of Bahmni and this release is critical for us to upgrade the log4j on all Bahmni repo’s. We will raise the PR with latest log4j 2.17.1 version if it helps.
Unfortunately, it’s not quite as easy as that. The changes to get OpenMRS to be able to use Log4J2 in place of Log4J1 were fairly extensive and need re-evaluation for how they apply to Core 2.1.x. I.e., this wasn’t simply a matter of swapping out libraries.
At the heart of it there are two issues:
OpenMRS allows (some) logging configuration to be driven by global properties (log.level, log.layout, and log.location). Support for this is implemented in a somewhat invasive way. Replicating this with Log4J2, which uses a fundamentally different configuration system was a challenge and, in retrospect, I didn’t do things quite correctly hence, this PR I’m actively working on.
OpenMRS uses an appender that stores log messages in-memory. This needed to be completely re-written. (This is called the MemoryAppender)
So the actual implementation of Log4J2 in OpenMRS is spread across several commits:
3e05209 (A more reliable way of getting the MemoryAppender)
7a1be48 (Leveraging changes in 6b0651b to more easily get the MemoryAppender)
And the Legacy UI module:
a926a70 (Added a hack to be able to load messages from the MemoryAppender via reflection on appropriate versions of OpenMRS)
I don’t think these changes can just be backported and released as an existing version, because we’d break things for anyone who was using a non-updated 2.1.1. Alternatively, I suppose, alternative pathways could be devised to load the MemoryAppender in 2.1.1 depending on the presence of a getMemoryAppender() method, but that’s not built into the existing PRs and would need to be new development work.
The easiest path forward I can see is to get Bahmni running against Core 2.1.4 (the changes for which can be found here… I haven’t extensively reviewed them). Then we could maybe work on backporting the Log4J2 patches to the 2.1.x branch and release that as 2.1.5 at least that way we can make suitable changes to at least the REST API to be able to load the server log or, if that’s not a feature Bahmni uses, I suppose we could skip it altogether.
I’m, of course, willing to supply whatever support and guidance I can on backporting these patches, but I likely don’t have enough time to take it on as a project myself. It might also be easier to look at the state of things once TRUNK-6052 gets merged in as I think that makes a lot of useful changes that should’ve been part of the initial implementation (hopefully it will be ready by the end of the week).
Thanks much @ibacher for the detailed commits list. This did help us a lot to start with the log4j changes on 2.1.4 OMRS version. We are still working on the log4j changes for both legacy UI and REST modules.
@burke Yes… forward-porting is usually more straight-forward that backporting.
The bigger issue around this is that this change will definitely break code (which is why I didn’t initially backport it). E.g., this code from Iniz, or this code from PIH. IIRC, calls to setLevel() should continue to work, but anything that attempts to create an appender is either a no-op (like logger.addAppender()) or will throw an exception, e.g., by attempting to create a new non-console appender.