Falco hogs all memory and crashes server upon invalid yaml

[MprBol]

Describe the bug

Upon invalid yaml, the falco process may hog all available memory by calling brk() -- resulting in unresponsive server due to all memory+swap+cpu being used by the falco process.

How to reproduce it

Create the following macro

- macro: exclude_commands
  condition:
    (user.uid = 1001 and user.loginuid in (-1, 1001) and proc.exepath = "/usr/bin/script" and proc.args in (
"/usr/local/bin/script_metrics.sh",
    "-c rsync"
    )  )

(Note the indentation error in the yaml above)

And start falco

Expected behaviour

Falco to error with the invalid yaml

Screenshots

Environment

• Falco version:

  {"default_driver_version":"7.2.0+driver","driver_api_version":"8.0.0","driver_schema_version":"2.0.0","engine_version":"40","engine_version_semver":"0.40.0","falco_version":"0.38.1","libs_version":"0.17.2","plugin_api_version":"3.6.0"}

• System info:

$ falco --support | jq .system_info { "machine": "x86_64", "nodename": "dddd.net", "release": "4.18.0-553.8.1.el8_10.x86_64", "sysname": "Linux", "version": "#1 SMP Fri Jun 14 03:19:37 EDT 2024" }

• OS: RHEL8.10
• Kernel:
• Installation method: RPM Additional context

[fbs]

I did some more testing to minimize the reproducer:

---

- macro: exclude_commands
  condition: x
"a"

LOAD_ERR_YAML_VALIDATE (Error validating internal structure of YAML file): Rules content is not yaml

---

- macro: exclude_commands
  condition: x
"a"

LOAD_ERR_YAML_VALIDATE (Error validating internal structure of YAML file): Rules content is not yaml

---

- macro: exclude_commands
  condition: x
a,

LOAD_ERR_YAML_VALIDATE (Error validating internal structure of YAML file): Rules content is not yaml

---

- macro: exclude_commands
  condition: x
"a",

Thu Jul 18 20:28:39 2024: Loading rules from file /etc/falco/falco_rules.yaml
Thu Jul 18 20:28:39 2024: Loading rules from file /etc/falco/falco_rules.local.yaml
... OOM

[MprBol]

Hi this bug could cause critical impact on a server landscape. Can someone confirm/have a look at this report? Thank you!

[sgaist]

Hi,

Thanks for the report !

Some questions to help debug:

• How are you starting falco ? (manually, automatically ?)
• What parameters are you using ?

I just tested with a fresh build of the latest code and it properly fails when loading when called from the command line. On your side, does it also get OOM killed if started manually ?

[MprBol]

hi @sgaist , thank you for replying!

Just simply starting falco --dry-run manually also triggers this mistake.

When you tried reproducing it, did you place

- macro: exclude_commands
  condition:
    (user.uid = 1001 and user.loginuid in (-1, 1001) and proc.exepath = "/usr/bin/script" and proc.args in (
"/usr/local/bin/script_metrics.sh",
    "-c rsync"
    )  )

in your rules.d directory? Including the wrongly aligned spacing for "/usr/local/bin/script_metrics.sh", ? It must be in this incorrect yaml to trigger this bug.

[LucaGuerra]

/milestone 0.39.0

[sgaist]

It looks like there was something wrong with my checkout and I was able to reproduce the issue.

[FedeDP]

It seems a yaml-cpp bug to me; did anyone report it to them?

[fbs]

Yes, it has been reported but not resolution yet

[FedeDP]

So, i was trying to reproduce this with a small example using just yaml-cpp but i could not. I will try to understand whether this is actually a Falco bug.

[FedeDP]

Oh i was wrong, the bug lies in YAML::LoadAll(std::string input) method from yaml-cpp; this example reproduces the bug without Falco involved:

#include <yaml-cpp/yaml.h>
#include <iostream>

int main() {
    static const std::string yml = R"(
- macro: exclude_commands
  condition:
    (user.uid = 1001 and user.loginuid in (-1, 1001) and proc.exepath = "/usr/bin/script" and proc.args in (
"/usr/local/bin/script_metrics.sh",
    "-c rsync"
    )  )  
    )";
    try {
    auto config = YAML::LoadAll(yml);
    } catch(const YAML::BadFile& e) {
        std::cerr << e.msg << std::endl;
        return 1;
    } catch(const YAML::ParserException& e) {
        std::cerr << e.msg << std::endl;
        return 1;
    }
    return 0;
}

@fbs can you share the upstream bug link? I will add this detailed info ;)

[FedeDP]

Opened 2 upstream PRs:

• https://github.com/jbeder/yaml-cpp/pull/1318 rejects corner cases scalar values that were instead accepted (the yaml that are causing this issue will basically start throwing yaml-cpp ParserExceptions since they are not correct yaml)
• https://github.com/jbeder/yaml-cpp/pull/1319 avoids the infinite loop in LoadAll() if something goes wrong in the parser (that's exactly what happened: since the wrong syntax was accepted, the parser was then not able to tell the current token type and thus it pushes a null node to the list of current nodes without consuming the current token, that is indefinitely re-parsed)

[FedeDP]

Waiting for upstream to merge last PR (https://github.com/jbeder/yaml-cpp/pull/1319) and eventually tag a new yaml-cpp release before updating. These fixes won't end up in the 0.39.0 Falco release, since we haven't got much time left.

/milestone 0.40.0