Skip to content
This repository has been archived by the owner on Jan 19, 2024. It is now read-only.

[CVE-2021-44228] upgrades logging to log4j 2.16.0 to mitigate RCE #134

Conversation

roechi
Copy link

@roechi roechi commented Dec 16, 2021

Fixes #133

This addresses RCE vulnerabilities expressed through CVE-2021-44228 and vulnerabilities which exist for prior versions of log4j 1.x

I was not able to run all tests since I was not able to figure out all requirements for my build environment.

@@ -37,7 +37,7 @@
<gson.version>2.3.1</gson.version>
<junit.version>4.12</junit.version>
<lang.version>2.6</lang.version>
<log4j.version>1.2.17</log4j.version>
<log4j.version>2.16.0</log4j.version>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<log4j.version>2.16.0</log4j.version>
<log4j.version>2.3.1</log4j.version>

Copy link

@gmazza gmazza Dec 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would much rather FuelSDK be updated to require Java 8 (in which case it should be 2.17.0), but if it is still to be Java 6, then from Log4J page: https://logging.apache.org/log4j/2.x/index.html, quote: "Mitigation Upgrade to Log4j 2.3.1 (for Java 6), 2.12.3 (for Java 7), or 2.17.0 (for Java 8 and later)."

If we use 2.3.1 here in order to maintain JDK 6 support, downstream users of this library using JDK 8+ will probably be able to override it to 2.17.x anyway.

Copy link
Author

@roechi roechi Dec 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmazza Many thanks for your suggestion! I agree, I would also rather FuelSDK be upgraded to a newer Java version, but on the other hand, some people rely on it running on Java 6. Using log4j 2.3.1 for continued compatibility might be the right call here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmazza However, this change alone wouldn't be enough: I introduced the Log4j 1.2 API bridge to keep the amount of required changes low. This package however, is not compatible with Java 6 (afaik).
I see two possible paths here:

  • use Log4j 2.3.1 to keep support for JDK 6; then we have to update all logger interactions in the project, since the logging API has changed
  • use the newest version of the Log4j 1.2 bridge to keep the amount of changes low but push the minimum JDK version for the project to version 8

I feel like the first option might be best.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roechi It looks like that JDK 6 declaration was not used in a release version of this application (i.e., 1.5.0). The last patch done in February bumped the release version to 1.5.1 and it was that version that declared JDK 6. It doesn't appear 1.5.1 was ever released.

I put in a support ticket with Marketing Cloud for them to look at this patch but they rather quickly closed it, they're firm that this is a community project and they don't provide official support for it. From what I can see, the last release was back in 2017 and this code has gotten quite old.

Anyway, my fork: https://github.com/gmazza/FuelSDK-Java is a copy of what we're testing at work, it uses the very latest CXF 3.5.0 released this month instead of a 2015 version in the main Salesforce branch, and latest 2.17.1 of Log4J. Code is quite new but initial testing is indicating that for obvious functionality it seems to be doing its job (I expect some tripping up sooner rather than later, however, I'll try to keep my fork updated as I find things.) There are numerous @ignores I needed to add to the test cases however, I suspect the API in Salesforce has changed for several of them (meaning even the main branch would have these failures), also that I might not have provided all needed configuration from the fuelsdk.properties.template file. Feel free to fork, see the fuelsdk.properties.template file on config needed to run tests via mvn clean install, and if you have a local Artifactory repository you can configure the pom.xml for them and do "mvn clean deploy" to install your local version and work with that.

@roechi
Copy link
Author

roechi commented Jun 10, 2022

Fix released with v1.6.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CVE-2021-44228] zero-day in the Log4j Java library
2 participants