Skip to content

Conversation

@StoneLabs
Copy link
Contributor

Using rolling logs previously prevented the CVE-2021-44228 mitigation from working.
This commit forbids rolling logs if required to prevent CVE-2021-44228.

This can be considered a hotfix.

See #1189

@StoneLabs
Copy link
Contributor Author

Tested for ENABLE_ROLLING_LOGS=TRUE and FALSE for 1.12 and 1.18:

root@4125eb90c629:/# export VERSION=1.18
[...]
SETUP_ONLY: java -Xmx1G -Xms1G -Dlog4j.configurationFile=/data/log4j2.xml -Dlog4j2.formatMsgNoLookups=true -jar 

root@4125eb90c629:/# export VERSION=1.12
[...]
[init] ERROR: Using rolling logs is currently not possible in the selected version due to CVE-2021-44228

root@4125eb90c629:/# export ENABLE_ROLLING_LOGS=FALSE
[...]
SETUP_ONLY: java -Xmx1G -Xms1G -Dlog4j.configurationFile=log4j2_112-116.xml -jar minecraft_server.1.12.jar

root@4125eb90c629:/# export VERSION=1.18
[...]
SETUP_ONLY: java -Xmx1G -Xms1G -Dlog4j2.formatMsgNoLookups=true -jar minecraft_server.1.18.jar

@StoneLabs
Copy link
Contributor Author

wait, i PR'd the wrong code xD one second please

@StoneLabs StoneLabs marked this pull request as draft December 12, 2021 10:42
Using rolling logs previously prevented the CVE-2021-44228 mitigation from working.
This commit forbids rolling logs if required to prevent CVE-2021-44228.

This can be considered a hotfix.
@StoneLabs StoneLabs marked this pull request as ready for review December 12, 2021 10:49
@StoneLabs
Copy link
Contributor Author

My bad. I've amended the correct code to the commit.
Should be good now.

Copy link
Owner

@itzg itzg left a comment

Choose a reason for hiding this comment

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

This is a great fix at this point. Someday I want to add a sub command to https://github.com/itzg/mc-image-helper to modify the application log4j config and insert the rolling log config.

@itzg
Copy link
Owner

itzg commented Dec 12, 2021

This has been merged and pushed to all the image variants

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants