javalog4j

Why is concatenation in log4j prone to attacks?

Here: https://logging.apache.org/log4j/2.x/manual/getting-started.html#best-practice-concat

They say this about using string concatenation in log4j:

More importantly, this approach is prone to attacks! Imagine userId being provided by the user with the following content: placeholders for non-existing args to trigger failure: {} {} {dangerousLookup}

Could someone explain why this is dangerous? I don't get what string concatenation has to do with this "dangerousLookup" thing.

Solution

  • I think the documentation is very clear

    Imagine userId being provided by the user with the following content: placeholders for non-existing args to trigger failure: {} {}

    In case you have the following logger and you pass the above userId

    LOGGER.info("failed for user ID: " + userId);

    The code execution will break since the logger will have as content the "failed for user ID: placeholders for non-existing args to trigger failure: {} {} " and the logger will throw an exception that the 2 arguments which are required, are not provided.

    Also as the commenter Violet mentioned someone could even pass an argument with concatenation which is evaluated dynamically. Something that should not be printed in the logs, e.g ${env:SECRET_ENV_VAR} which is the case mentioned for dangerous lookup.

    You can also combine the above two for example and see how someone could use a thrown exception from a logger that provides information for the exception to the user to get information that should be otherwise hidden.

    This is why they advice as requirement that the layout of the message is concrete and not in control of the argument. E.g.

    LOGGER.info("failed for user ID `{}`", userId);

    Now the logger will just print the provided argument userId as described in the layout and will not mix it with the layout so that you are protected from the above and several other issues.