Hacker News Reader

Comment by stefan_ | Hacker News Reader

There seems to be a misunderstanding here. We have on the one side a garbage feature that should never have been implemented - but if you want to keep it for backwards compatibility, sure. But then we have log4j scanning all values instead of only format strings - I think it can be argued that this behavior is a critical bug and was never intended to begin with. It seems to have only come about because whoever implemented the JNDI stuff lost their bearing in the absurd class hierarchies and abstractions in log4j.

Of course the last part holds the solution for our backwards compatibility issue. Remove the JNDI nonsense from the default package and move it into an extension package. Whoever wants to keep it can just add that to their dependencies and continue to enjoy logging functions that sometimes also make network connections and block your program.

View on HN

Indeed - as evidence for this, I would submit that slf4j and logback were created to offer a drop in replacement for log4j (slf4j literally provides alternative implementations of the org.apache.log4j.Logger class), but I have never seen anybody complain that "I switched to logback and slf4j and my jndi substitutions stopped working."

Nobody thought this was how log4j worked; log4j's documentation for format syntax only covers {} placeholders - the same format that slf4j has grandfathered in from log4j.

I agree this feels like a case where they got confused about their internal terminology. Log4j refers to messages with {} placeholders as 'FormattedMessages'; it refers to the log pattern syntax as 'Patterns' in code - but it seems to refer to them as 'log formats' in documentation.

Somewhere in this mess, someone hooked up the pattern capabilities into the formatting system.

> but I have never seen anybody complain that "I switched to logback and slf4j and my jndi substitutions stopped working."

SLF4J was created to replace Apache Commons Logging and Logback was created to replace Log4j 1.x. Both were created Ceki Gülcü, the original author of Log4j 1.x [1].

Logback came out in 2006. The first beta version of Log4j 2.x was only released 6 years later in 2012, and the JNDI lookup feature was added in 2.0-beta9[2] in 2013!

Obviously nobody complained when switching from Log4j 1.x to SLF4J+Logback that a feature from a completely different library (with the same name) that would be created 7 years into the future was not supported.

> Somewhere in this mess, someone hooked up the pattern capabilities into the formatting system.

That's not what happened. The lookup mechanism (which includes "${jndi:}" lookups) is completely unrelated to the message formatting subsystem.

The way formatting and pattern lookups work in log4j2 is:

1. logger.info("Hello {}", "world") creates a FormattedMessage instance with the "Hello {}" format string and a single parameter, "world".

2. The FormattedMessage is wrapped in a LogEvent and routed to the correct appender(s).

3. Most appenders will format the LogEvent with a Layout. In our case, it's PatternLayout we care about[3].

4. PatternLayout will pre-calculate a set of PatternConverters based on your pattern, so it doesn't have to keep parsing the pattern on every invocation. "%m" will map to MessagePatternConverter.

5. (grossly simplifying zero-garbage and streaming optimizations) Each pattern converter is executed and appends to the final layout text's StringBuilder.

6. (grossly simplifying oh so many things) MessagePatternConverter will first call event.getMessage().getFormattedMessage(). The logic for formatting the message is entirely encapsulated by Message and its subclasses. MessagePatternConverter has no way to distinguish the format string from the user-provided parameters!

7. MessagePatternConverter finally applies the pattern lookups to the formatted message text. The pattern lookup mechanism is completely separate from and orthogonal to the message formatting mechanism.

---

That was long-winded, but I had to fight these annoying misconception about "log4j not implemented format strings properly".

Now, there are several things I'm not saying here:

1. I don't think more than a handful of people ever relied on lookups working on the log message (formatted or otherwise), as opposed to the pattern in the configuration file.

2. I don't think Log4j should have kept compatibility here. The moment the maintainers implemented "%m{nolookups}" (on version 2.7), they should have made it the default. That being said, I know this is very hard to do in the Java ecosystem. But I think it is time that the Java developer community changes its extremist position regarding compatibility at all costs.

3. I don't think that Log4j should have implemented pattern lookups for text messages to begin with. Even if was just the format string part (which is impossible to do with Log4j's current architecture anyway).

4. I don't think any kind of string formatting should be included in a logging library. If you want to format log messages, use an external formatting function or string interpolation (if you're lucky enough to be using Kotlin or Scala). If it is added, it should only be used as a convenience, and shouldn't do anything more than formatting (like lookups). Relying on developers to always remember that log.info("Hello {}", world) is safe and log.info("Hello {}" + world) gives the entire internet full control of your server is beyond stupid. Even if Log4j went with this silly distinction, I would say it was a horrible design.

[1] https://techblog.bozho.net/the-logging-mess/

[2] https://logging.apache.org/log4j/2.x/changes-report.html#a2....

[3] It seems like PatternLayout is the only layout vulnerable to this bug in log4j2, but it is hard to tell, the implementation being a classic Java mess of deep class hierarchy, liberal use of reflection to control everything and some heroic attempts to break SOLID principles at least 4 times on a single line of code. Take my analysis with a grain of salt. It's a gross simplification of what is unfortunately par for the course in many Java libraries.*

Thanks for bravely diving deeper into the class hierarchy than I did.

Your analysis of what should have been done in 2.7 is spot on.

Regarding my point about migration compatibility - I would not assume that the only time anyone has moved from log4j to slf4j was when slf4j first came out. Slf4j+log back is also a drop in replacement for log4j 2, up to a point.

Thank you for your kind words!

> Regarding my point about migration compatibility - I would not assume that the only time anyone has moved from log4j to slf4j was when slf4j first came out. Slf4j+log back is also a drop in replacement for log4j 2, up to a point.

In a way, you're right. By themselves, SLF4J+Logback are not drop-in replacement for Log4j 2.x (or Log4j 1.x for that matter), but Log4j 2.x does provide an adapter for that sends all the calls to log4j-api interfaces to whatever is implementing SLF4J (e.g. logback). It also provides an API that does the reverse (implementing SLF4J through whatever is implementing the log4j-api). To top that up are also adapters which converts Log4j 1.x API, the JUL APIs and Apache Logging Commons (yet another facade, like SLF4J or log4j-api 2.x) to SLF4J or and log4j.

Accounting for all the permutations there, there are probably thousands of slightly different migration paths that you could take, and they're all slightly different. This makes the situation a lot more complex than it seems, at first hand.

I think you're imagining a project which started with Log4J 2.x and moved to SLF4J+Logback. You're right that such projects may exist, but to be more accurate:

1. By the time Log4J 2.x has started, SLF4J was already established as the standard facade for Java logging libraries. Log4J provided an SLF4J binding from the get go, and I think many (if not most) projects which ended up using Log4J are using it through the the SLF4J binding.

2. By the time Log4J 2.x started getting popular (around 2014-5?), Logback Development slowed down. It was abandoned per se. There were still or or two minor releases a year until 2018, so it didn't die out, but showed slow progress. At the same time Log4J 2.x was adding new features quickly and making some impressive performance gains in multithreaded workloads[1]. So while there were some reasons to move from Logback to Log4j 2.x, there were no strong reasons to do the reverse.

In short, I don't think many people every migrated between them.

There is a better argument you can make of course: Log4j 2.x just removed supports for message lookups completely and no one complained. It shows that they could have just done it years ago with little worries. But we need to work harder to change the "compatibility über alles" mindset that prevails in Java and other ecosystem. It's perfectly ok to break compatibility for 0.001% of your users when you've got a serious security issue. Punishing 99.999% of the other users with an RCE because 0.001% MIGHT rely on some back is not good engineering!

[1] https://logging.apache.org/log4j/2.x/manual/async.html

> But then we have log4j scanning all values instead of only format strings - I think it can be argued that this behavior is a critical bug and was never intended to begin with.

It was actually intended behavior, and this is what really boggles the mind! Javadoc says explicitly that variable replacement is recursive, with cycle detection (which will throw! What happens to the log line in this case?) [0].

[0] https://logging.apache.org/log4j/2.x/log4j-core/apidocs/org/...

That link is about variable replacement in config strings, which is intentionally recursive. It doesn't mention the use of the variable replacement mechanism when interpolating values into log messages, which is what makes this vulnerability so bad, and as far as I can see was not intentional.

Right, I was also confused by the blame on backward compatibility. You can keep things backward compatible without necessarily making it on by default. There is no reason why `formatMsgNoLookups` should the default. If it is indeed an obscure and hacky feature for backward compatibility, just make it opt-in. People who really care about it will enable it, most people won't have to carry that baggage and we wouldn't be in a situation like this.

Hindsight is always 20-20.

>for a feature we all dislike yet needed to keep due to backward compatibility concerns.

If they really dislike the feature that much, they likely dislike the code and want to completely delete it. I'm not sure if making it opt-in would make them as happy as fully deleting it, so they are less motivated to make it opt-in than they would be to fully delete it.

They could also "fully" delete it by putting it in a separate opt-in package.

"lost their bearing in the absurd class hierarchies and abstractions" sounds familiar. Java app stack traces are like Neal Stephenson epics, but less entertaining.

And enabled by default. That's the most mind-blowing bit of this feature. The backcompat argument is a deflection for shipping a time bomb into people's codebases.

To be fair to the maintainers, they didn't ship anything into people's codebases. People chose Log4j and pulled it into their code. FOSS contributers aren't responsible for downstream use of their projects.

I don't think this argument makes much sense, beyond completely legalistic.

If you add backdoor to the used library, then yes you shipped it there. You don't have legal liability for consequences when it is found, but it does not make not being the one who shipped it.

It just protects you legally, as it should.

> I don't think this argument makes much sense, beyond completely legalistic.

The downstream users chose to use Log4j, chose to upgrade to the version where the exploit was introduced, chose not to audit the code. It's their responsibility and theirs alone.

A maintainer can release a new version of their software. No-one is under any obligation to use it. They certainly don't reach down into your repos and push their changes.

So I agree that the maintainers shipped a release. I certainly don't agree that the shipped it into anyone else's codebase.

> The downstream users chose to use Log4j, chose to upgrade to the version where the exploit was introduced, chose not to audit the code. It's their responsibility and theirs alone.

No. It is manipulative framing where responsibility for backdoor is pushed onto users.

It is especially ridiculous in the context of industry where not upgrading is seen as irresponsible. And especially ridiculous when the same industry pushes for open source and when companies eventually start to listen and use open source, acts like there was something wrong with using open source.

Totally agreed. If software is ever to become a serious and respected industry we can't just deny responsibility and blame the users as soon as something goes wrong.

Why don’t all those people affected by this ask for their money back?

You can already do like in real industries, pay someone to write you a logging framework and they will be responsible for it. Guess what, you can even pay someone to just be responsible and audit FOSS code for you. Looks like we are already a serious and respected industry that just contains a few complaining entitled users benefiting from the free work of others...

> No. It is manipulative framing where responsibility for backdoor is pushed onto users.

I fundamentally disagree that anything was pushed onto downstream users. They explicitly pulled Log4j.

Framing it as the maintainers pushing the update onto downstream users is itself so manipulative that I wonder if you are conversing in good faith. Framing the bug as a backdoor is absurdly manipulative.

The Apache License even warns the users explicitly:

> Unless required by applicable law or agreed to in writing, Licensor provides the Work (and each Contributor provides its Contributions) on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied, including, without limitation, any warranties or conditions of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A PARTICULAR PURPOSE. You are solely responsible for determining the appropriateness of using or redistributing the Work and assume any risks associated with Your exercise of permissions under this License.

You are the part of the problem then.

Lack of legal liability for any kind of security exploit, and auditing processes for companies that allow wild west integration of libraries are the problem.

Dude, take a step back here. You're using someone else's free code. Someone wrote some code for free, put it online, and said "yeah anyone can use this for free go ahead". They don't owe anyone anything.

>It is especially ridiculous in the context of industry where not upgrading is seen as irresponsible.

Upgrading without due diligence, and without reason is as irresponsible an not upgrading when the need to becomes manifest.

Running "unsafe" versions of software in conditions where their unsafe nature is not open to exploitation is a completely legitmate action. If you upgrade just because there's a later version available, you're rolling the dice as needlessly as the fellow that'll get around to it once he has time to read the code.

Note: I'm aware of, and also cherish the no warranties explicit or implied clause. However, In real life, where social relationships do matter, I don't see this escape clause standing the test of time as society catches up the technology.

Maintainers have responsibility over their code, not how it is integrated. Here the problem is entirely in their code, it is not depending on the downstream project or any way it is used there.

Maintainers have no responsibility at all, unless they're paid or bound by contracts in some other way.

Some feel responsible regardless, but they certainly don't have to. They can even introduce vulnerabilities intentionally, and it's your responsibility if you trusted them not to.

> They can even introduce vulnerabilities intentionally, and it's your responsibility if you trusted them not to.

Is that true? I can add code in my open source library to steal credit card numbers, and if you use it that'd be your fault?

There is a reason why some companies have internal repos only policy, and libraries only get added to them after legal and IT review.

It's not true. There's a legal system and they cannot do intentionally illegal things and fraud.

But it's good to not trust random strangers at GitHub -- maybe a user profile is just a facade for a criminal gang, maybe untraceable so they can get away with it

Not every intentional vulnerability is meant for illegal things or fraud.

If you publish something, you have responsibility over it.

Imagine the liability of publishing any code if you are "responsible" for it. Meaning that you would be responsible for its improper use, or even use for illegal purpose.

You are though. Forget the legalese for a second.

You wrote the code and put it out there.

If you didn't, none of the uses of it would ever have come to fruition.

A court of law may let you duck out for the time being, but when you trace the chain of effective physical causality backwards, at the end of the day, you write it, you're responsible for making possible it's applications.

There is value to the code unwritten. That value is a clean conscience and true abscence of guilt or remorse for having enabled someone to do something monstrous.

I laugh at people who think a legal disclaimer absolves one of moral culpability. If only it were that easy.

I wonder how do the makers of kitchen knives sleep at night.

No, I don't. You may want to read the licenses of the code you're using, by the way.

What kind of responsibility? In what sense? Could you give me an example?

By the way, this is a part of my license (a pretty common one):

  THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
  WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
  MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
  ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
  WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
  ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
  OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

No, you can't escape legal responsibility from intentional sabotage that easily.

Tell that to the people that removed their 'leftpad' repository.

Or the ones that are taking over FOSS project to inject 'telemetry' spyware

Intentional sabotage of my own project?

It doesn't take much imagination to come up with situations where one may intentionally introduce vulnerabilities in use-cases they don't care about in order to make handling of use-cases they do care about easier. Are you sure I can't "escape legal responsibility" for doing that in my own software that I share to others "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT? (emphasis not mine ;))

Sounds as if you believe you could edit & change open source code to try to intentionally crash a car or an airplane, or just not care about that happening, and get away with it, just because "AS IS".

If the usage terms say "do not use in any critical applications", I would've thought that the responsibility for using the code in that fashion woudl be squarely with the entity that did the integration?

It'd probably be better if the usage terms would by necessity spell out that you were happy for the code to be used in life-critical situations, instead of having to opt out of it.

The default usage terms are "all rights reserved, nobody can use this but me". You change this by applying licenses which regulate the terms under which you're happy for the code to be used by others. The vast majority of popular Free Software licenses allow you to use the code under no guarantees whatsoever, so if you want to use some software in critical applications and hold its authors responsible if it doesn't work as advertised, you should probably pay them and include this responsibility in their contract.

Irresponsibility != malice.

If your FOSS software uses plaintext passwords because you don't care about data-at-rest security for whatever reason, sure, you're not required to make all your public code super secure.

Otherwise all student projects uploaded to GitHub would be crimes.

If your FOSS software adds a bit of code that POSTs all inputted credit card numbers to https://seba_dos1.com, well, that's gonna look very different in court.

It's like holding a garage sale where you give stuff away for free. Nobody can complain if your old stereo that you gave away for free doesn't work, but if you have an old propane burner that's likely to sear somebody's face off, best to just throw it away.

Exactly what part of:

THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

Suggests that maintainers have any responsibility over their code?

It's not obvious that putting in this legalese covers you if you otherwise promote it as working software and invite people to use it, and details would depend on jurisdiction.

Has Apache 2.0 ever been pierced in court anywhere?

There's a legal system too; that license text isn't the whole world.

However in this case, to me it seems they obviously have nothing to worry about. (Since this bug was unintentional)

They wrote code essentially willing to run exec on any string passed to it, enabled this feature by default, and then didn't loudly warn people that any string passed to log4j must be trusted. None of this is competent work.

Surely then the responsibility lies with the people who decided to use this apparently incompetent work in their infrastructure?

If I release terrible software with a license that expressly has no warranty and someone uses it, surely that's on them?

You must audit every single line of code for every piece of software you use.

It’s the equivalent of shipping a car with faulty brakes. It’s really debatable who is responsible.

Reply on news.ycombinator.com