Re:Log4j RCE Found
by janekm on Friday December 10, 2021 @07:51AM
(#29509260)
Though of course "debug.log(stuffIGotFromPeer)" is also very common (and as you point out should always be avoided).
[ Parent
| on HN ]
If "debug.log(stuffIGotFromPeer)" is dangerous, then the problem is with debug.log and not my code.
Safe logging is not too high of a bar.
The point is that the first argument is the format string. debug.log("{}", stuffIGotFromPeer) should generally be safe (this bug is an example of when it isn't, though)
The fact that the first argument is interpreted as a format string even when you aren't supplying format arguments is a violation of 'principle of least surprise'. The availability of format parameters that can access environment data - let alone remotely loaded code - is a feature most people won't discover unless they go looking for it.
How are varargs implemented in the JVM ABI? Is the called function even aware of how many parameters were actually passed, or does it have to rely on the format string for that?
Turned into an object[] array.
The log4j API implements a few overloads of each log method to help avoid the implicit array allocation happening in common logging cases (no args, one arg, etc).
The call site can tell if args were provided.
The format string should not allow remote execution unless explicitly tuned on either.
Logging should not be the thing you have to sanitize.
TBH, this is a huge antipattern. You have to put some context in your logs, so it must be something like "debug.log("Got from peer: {}", stuffIGotFromPeer) or something like that. You can't complain printf(stuffIGotFromPeer) is dangerous, so here is the same. But in log4j, debug.log("Got from peer: {}", stuffIGotFromPeer) is as unsafe as the other one, too! There's no way to make it safe, as I understand.
Don't see why not. Evaluate the `${}` stuff first, then do the `{}` substitutions.