[CVE-2021-44228]: Update Log4J to resolve security issues

[Gamebuster19901]

Some downstream projects may be vulnerable if they don't explicitly define what log4j version to use themselves, best to update log4j for any downstream projects that may still use LegacyLauncher.

[craftycodie]

Well spotted

[Pokechu22]

Updating log4j2 across several major versions like this is likely to cause issues (Mojang has actually not updated log4j when they mitigated the issue). Projects probably also won't automatically update to the new version of LegacyLauncher either.

I think it would be better to update the project's log4j2.xml to the new version provided here, though that also won't automatically be deployed.

[craftycodie]

@Pokechu22 given this project's minimal use of log4j I kinda doubt it

[Pokechu22]

For this project, sure, but if the goal is to update log4j so that dependencies on this project get a newer log4j version, that could cause issues.

[craftycodie]

Perhaps, there's not many downstream dependencies here, and my concern would be game interaction, but I'll compile & test

[Gamebuster19901]

If there are unforseen issues with this PR, let me know and I'll close it.

[craftycodie]

I setup beta 1.7.3 with this and it was fine

That said I don't think a launchwrapper update is actually necessary, I think you can just bump the dependencies in the launch manifest json files.
Additionally, slf4j18-impl is unnecessary, doesn't need to be added.

[Pokechu22]

Note that beta 1.7.3 (and in fact 1.5.2, the latest vanilla version to use launchwrapper) use version 1.5, which predates the version that added log4j (launchwrapper 1.9, I think), so they shouldn't be affected. It's mods that use newer versions, to my understanding.

[craftycodie]

Yes but this PR would bump to 1.12 which includes log4j

[Marcono1234]

Would probably be good to update the Log4j version either way (for future users of this library). But as mentioned by Pokechu an additional fix might be needed for those who cannot easily update the Log4j version?

[MisterSheeple]

Why is this closed? This is still an issue that should be addressed. It's a zero-day, for crying out loud.

[Gamebuster19901]

Why is this closed? This is still an issue that should be addressed. It's a zero-day, for crying out loud.

Well, as stated in Mojang/DataFixerUpper#60, it seems that mojang is switching to slf4j. But I guess I'll reopen until a fix is pushed here.

[Gamebuster19901]

Note: This still needs testing on game versions which contain log4j. Additionally, only updating to log4j 2.16+ will fully resolve all known vulnerabilities. See https://www.lunasec.io/docs/blog/log4j-zero-day/

[MisterSheeple]

Actually does Mojang even listen to PRs anymore? They're probably more likely to come up with their own solution for this, but they don't even seem to be aware of this vuln existing in LegacyLauncher too. All official docs say only 1.7-1.18 are affected.

…

On Fri, Dec 17, 2021, 10:03 PM Gamebuster ***@***.***> wrote: Note: This still needs testing on game versions which contain log4j. — Reply to this email directly, view it on GitHub <#34 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGS7E2HMKHNZWSME3OI3REDURP2XRANCNFSM5JZQP62Q> . You are receiving this because you commented.Message ID: ***@***.***>

[Pokechu22]

Quoting myself here:

Note that beta 1.7.3 (and in fact 1.5.2, the latest vanilla version to use launchwrapper) use version 1.5, which predates the version that added log4j (launchwrapper 1.9, I think), so they shouldn't be affected. It's mods that use newer versions, to my understanding.