Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace log4j2 with tinylog2 + slf4j bridge #8007

Closed
Siedlerchr opened this issue Aug 20, 2021 · 13 comments · Fixed by #8226
Closed

Replace log4j2 with tinylog2 + slf4j bridge #8007

Siedlerchr opened this issue Aug 20, 2021 · 13 comments · Fixed by #8226
Labels
build-system dependencies Pull requests that update a dependency file Logging

Comments

@Siedlerchr
Copy link
Member

We always get this Logger error messages on startup and we use a snapshot version of log4j2 which will probably take a long time until it's stable.

We already use slf4j as a Logger interface, so switching to another underlying logger should not be that complicated. Most stuff should be compatible.
We currently use slf4j-2.0.0. This requires at least tinylog 2.4

https://tinylog.org/v2/download/#slf4j

What needs to be changed?

  1. Replace log4j2 with tinylog and the bindings.
  2. Investigate if we still need our custom Logging Plugins or if they need to be converted https://tinylog.org/v2/extending/
  3. Remove log4j stuff also from jpackage/jlink
  4. Test if it works on all platforms
@Siedlerchr Siedlerchr added code-quality dependencies Pull requests that update a dependency file labels Aug 20, 2021
@koppor
Copy link
Member

koppor commented Aug 21, 2021

Historic remark: In 2017, I replaced our hole logging by jcabi framework at #3015.

Two concerns:

  1. The method signature for logging exceptions was very different ([WIP] Use jcabi-log to reduce startup time #3015 (comment))
  2. No SLF4J was used.

In last devcall we discussed that we want to keep slf4j. - For me, it would be OK to even go away from slf4j for logging our things in case this increases startup time.

@koppor
Copy link
Member

koppor commented Aug 21, 2021

This will probably fix #5475 (refs #5475 (comment)).

@ThiloteE
Copy link
Member

log4j seems to have a critical software issue. Germany's federal ministry for information security put it on code red:

@Siedlerchr
Copy link
Member Author

JabRef uses version 3.x snapshot. Gets automatically updated. And JabRef does not really log user input and is not running on a server.

@ThiloteE
Copy link
Member

ThiloteE commented Dec 14, 2021

Ah nice, this critical security issue seems to affect versions 2 .0 - 2.14 only. Not relevant for Jabref then :)

Edit: But what about Jabref 5.3? :O

We currently use slf4j-2.0.0. This requires at least tinylog 2.4

  • Koppor August 2021.

This would mean that Jabref 5.4 should be dropped soon, no?

@calixtus
Copy link
Member

We expect to be able to release 5.4 in the next few days / two weeks.
But to adress your concerns, I just checked 5.3: it's using log4j-3-snapshot too.

@navoooo
Copy link

navoooo commented Dec 15, 2021

And JabRef does not really log user input and is not running on a server.

It does not have to run on a server to be targeted, a compromised BibTeX/BibLaTeX database distributed by mail or downloaded could've been enough if JabRef was vulnerable. The file format is commonly exchanged by various means and is not generally suspected of malicious content, so if it were possible to load executable code through this with a well-placed string, it's not hard to imagine it being used as a delivery method for worms or ransomware targeting researchers.

It at least appeared to me as if there might be some candidates too, e.g. from bibfile directly, from PDF annotations and presumably user input, but I did not look into it further after finding the log4j version string.

@dengelt
Copy link

dengelt commented Dec 16, 2021

But to adress your concerns, I just checked 5.3: it's using log4j-3-snapshot too.

Does that really mean it's not vulnerable though? Isn't the "3.0.0-SNAPSHOT" version of log4j just whatever is currently on the master branch? At least I can't find any actual released versions with that version number anywhere, and nobody seems to mention it when talking about the log4shell vulnerability.

If that is the case, then anyone building JabRef before 12th December would have a vulnerable version (including the 5.3 binaries distributed on the release page that were built on 5th July).

@Siedlerchr
Copy link
Member Author

No idea though, but seems possible. There might be more differences between 2.x and 3.x. However, we are preparing a release in the next days which will then include the latest available version.
Despite that, the attack surface in JabRef is extremely low. I checked if I can trigger this while importing (e.g a modified bib file et but could not trigger the vuln..

@koppor
Copy link
Member

koppor commented Dec 16, 2021

Just for the record:

@cinderbdt
Copy link

cinderbdt commented Dec 16, 2021

Could you please comment on where on the Windows distribution one would replace the log4j2 or mitigate it?
Thank you.

@Siedlerchr
Copy link
Member Author

You would need to put the agent into the folder runtime\bin and modify the jabref.bat start script and add it to the command line arguments before the "-m" e
For linux and mac use the JabRef shellscript starter.

pushd %DIR% & %JAVA_EXEC% %CDS_JVM_OPTS%  -p "%~dp0/../app" -javaagent:path/to/log4j-jndi-be-gone-1.0.0-standalone.jar -m org.jabref/org.jabref.gui.JabRefLauncher  %* & popd

@calixtus
Copy link
Member

Good news, we just merged #8226 into the main branch, so in 5.4 Tinylog will be used instead of log4j.
We plan to release before christmas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-system dependencies Pull requests that update a dependency file Logging
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants