Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid log injection / log forging #1355

Open
orousseil opened this issue Jan 2, 2023 · 2 comments
Open

Avoid log injection / log forging #1355

orousseil opened this issue Jan 2, 2023 · 2 comments
Labels
enhancement New feature or request
Milestone

Comments

@orousseil
Copy link

orousseil commented Jan 2, 2023

Is there a configuration to avoid log injection / log forging ?

This is a security problem describe here : https://owasp.org/www-community/attacks/Log_Injection
In java, with Log4j2 or Logback, it can be avoid with special pattern like %encode (or %enc alias) or by defining conversionRule (for logback).

Look at detail solution here : https://github.com/augustd/owasp-security-logging/wiki/Log-Forging

@lamweili lamweili added the enhancement New feature or request label Jan 17, 2023
@lamweili lamweili added this to the 6.8.0 milestone Jan 17, 2023
@lamweili
Copy link
Contributor

lamweili commented Jan 17, 2023

At the moment, there is no crlf filter.
This is a good security feature to have. I will add it to the feature list and work on it once I get the time.

In the meantime, if you need it urgently, you can use tokens.

Change the default pattern %[[%d] [%p] %c - %]%m (replacing %m with %x{crlfFilter}).
It should look like %[[%d] [%p] %c - %]%x{crlfFilter}.

const log4js = require("log4js");
log4js.configure({
  appenders: {
    out: {
      type: "stdout",
      layout: {
        type: "pattern",
        pattern: "%[[%d] [%p] %c - %]%x{crlfFilter}", // default is %[[%d] [%p] %c - %]%m
        tokens: {
          crlfFilter: function (logEvent) {
            let filteredData = logEvent.data.map((i) => {
              if (typeof i.replace === "function") return i.replace(/\n/g, '\n> '); // add a > prefix
              else return i;
            });
            return util.format(...filteredData);
          },
        },
      },
    },
  },
  categories: { default: { appenders: ["out"], level: "info" } },
});

var logger = log4js.getLogger();
logger.info("Some debug messages\n[2023-01-17T11:58:38.150] [INFO] default - Log forging");
[2023-01-17T11:58:38.150] [INFO] default - Some debug messages
> [2023-01-17T11:58:38.150] [INFO] default - Log forging

@lamweili lamweili modified the milestones: 6.8.0, 6.8.1 Feb 20, 2023
@lamweili lamweili modified the milestones: 6.9.0, 6.9.1, 6.9.2 Mar 7, 2023
@OmarHawk
Copy link

OmarHawk commented May 3, 2023

At the moment, there is no crlf filter. This is a good security feature to have. I will add it to the feature list and work on it once I get the time.

In the meantime, if you need it urgently, you can use tokens.

Is this somehow also possible using a configuration file instead? For us, log4js is part of a vendor provided software where we cannot just add a token into an object based configuration but only have the option to pass the string path to a configuration file. But having a function is not not valid JSON... thus this line here will throw an error:

return JSON.parse(fs.readFileSync(filename, 'utf8'));
} catch (e) {
throw new Error(
`Problem reading config from file "${filename}". Error was ${e.message}`,
e
);

If you have any other idea to support this with config file only, please let us know as we would like to protect ourselves as well against log forging. ;-)


Apart from that:

Ideally, this filtering would be easily available not only as standard mechanism (removing the crlf completely), but also with a replace function, just as log4j2 (see https://logging.apache.org/log4j/log4j-2.1/manual/layouts.html#Patterns - replace ) or logback (https://logback.qos.ch/manual/layouts.html#replace) provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants