falco icon indicating copy to clipboard operation
falco copied to clipboard

new(rules): Directory traversal monitored file read

Open incertum opened this issue 3 years ago • 16 comments

Signed-off-by: Melissa Kilby [email protected]

What type of PR is this?

/kind feature

/kind rule-create

Any specific area of the project related to this PR?

/area rules

What this PR does / why we need it:

Add new rule Directory traversal monitored file read.

Web applications can be vulnerable to directory traversal attacks that allow accessing files outside of the web app's root directory (e.g. Arbitrary File Read bugs). System directories like /etc are typically accessed via absolute paths. Access patterns outside of this (here path traversal) can be regarded as suspicious. This for example will allow monitoring the entire /etc directory. For instance while /etc/passwd is not regarded a sensitive file like /etc/shadow - reading /etc/passwd via directory traversal is often performed as reconnaissance / probing for Arbitrary File Read vulnerabilities especially in web applications, because /etc/passwd is always there and it is world readable. In addition, vulnerability scanners will often try reading /etc/passwd via directory traversal ../../../../../../../etc/passwd.

Thanks @Kaizhe and @darryk10 for feedback to derive final version.

Example simulated log

{"priority":"Warning","rule":"Directory traversal monitored file read","source":"syscall","time":"2022-07-15T00:00:34.758753699Z", "output_fields": {"container.id":"host","container.image.repository":null,"evt.res":"SUCCESS","evt.time":1657843234758753699,"fd.name":"/etc/shadow","fd.nameraw":".././././.././././../././././../etc/shadow","proc.aname[2]":null,"proc.cmdline":"cat ../././//.////.././././.././././///./../etc/shadow","proc.cwd":"/home/vagrant/","proc.exepath":"/bin/cat","proc.name":"cat","proc.pname":"sudo","user.loginuid":1000,"user.name":"root","user.uid":0}}

Which issue(s) this PR fixes:

Special notes for your reviewer:

Needs new filter/display field fd.nameraw https://github.com/falcosecurity/libs/pull/468

Does this PR introduce a user-facing change?:

new(rules): Directory traversal monitored file read

incertum avatar Jul 08 '22 23:07 incertum

This is a really good one @incertum , do you think adding a condition about non shell parent process would make more sense? I am thinking a scenario a path traverse is an exploit through a web app.

Kaizhe avatar Jul 09 '22 05:07 Kaizhe

Thanks @Kaizhe. Absolutely can add the additional filter statement in and concur this rule is only relevant for web applications that have an Arbitrary File Read bug.

incertum avatar Jul 09 '22 05:07 incertum

@darryk10: changing LGTM is restricted to collaborators

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

poiana avatar Jul 11 '22 14:07 poiana

/milestone 0.33.0

jasondellaluce avatar Jul 12 '22 09:07 jasondellaluce

One liner to simulate if rule triggers python -c 'import subprocess; subprocess.Popen("cat ../../../////etc/passwd", shell=True)'.

incertum avatar Jul 12 '22 21:07 incertum

I'm also wondering if we could expand the /etc/passwd use case with more broad "Sensitive files" which can includes also other files attackers might want to read (ssh keys, sudoers file, etc). The macro sensitive_file_names has some examples. WDYT @incertum @Kaizhe ?

darryk10 avatar Jul 13 '22 09:07 darryk10

@darryk10 indeed, this seems more economic to just create one more complete rule, good call :) New suggestion below. Would you have additional or other ideas?

- rule: Directory traversal monitored file read
  desc: monitored file read via directory traversal
  condition: open_read and (fd.name="/etc/passwd" or sensitive_files or fd.directory="/etc/security" or fd.name contains ".ssh/") and fd.nameraw glob *../*../* and not proc.pname in (shell_binaries)
  enabled: true
  output: monitored file read via directory traversal (username=%user.name useruid=%user.uid user_loginuid=%user.loginuid program=%proc.name exe=%proc.exepath
    command=%proc.cmdline parent=%proc.pname file=%fd.name fileraw=%fd.nameraw parent=%proc.pname
    gparent=%proc.aname[2] container_id=%container.id image=%container.image.repository returncode=%evt.res cwd=%proc.cwd)
  priority: WARNING
  tags: [filesystem, mitre_discovery, mitre_exfiltration]

(1) Number of occurrences of a substring could be a neat future filter option. Meanwhile while not perfect matching for ../. and ./.. could perhaps be a better way to approximate at least 2 levels traversals for scenarios where you have ../././//.////.././././.././././///./../ which is being sanitized to .././././.././././../././././../ -> or actually maybe use the fd.nameraw glob *../*../* instead?

(2) .pem files and the like can be of interest as well, but since those belong more to the user's directory, relative paths with traversal are much more likely to be benign -> perhaps something end users may want to consider for custom local rules.

(3) Would need to place this rule before rule Read sensitive file trusted after startup as it would be the broader / more rich signal that should take precedence.

(4) Any additional output fields we should add?

(5) Failed file access attempts seem not to be logged -> likely need libs src adjustments for that.

incertum avatar Jul 14 '22 05:07 incertum

Hi @incertum, really good summary and I really like the condition you came up with. I agree with the fd.nameraw glob *../*../* in (1) and (3). Nothing to add in (4) the output looks good to me. I was just thinking that another usage of path traversal is to read the id_rsa (private key) to get SSH access to the host. What do you think on adding fd.name="id_rsa", to add the specific SSH private key use case?

darryk10 avatar Jul 14 '22 12:07 darryk10

Tweaked it a bit, proposing that it could be even simpler - just look for /etc any file. Also suspecting glob is a more expensive filter, fd.nameraw contains "../" and fd.nameraw glob *../*../* filter may give perf boosts.

# Web applications can be vulnerable to directory traversal attacks that allow 
# accessing files outside of the web app's root directory (e.g. Arbitrary File Read bugs). 
# System directories like /etc are typically accessed via absolute paths, 
# access patterns outside of this (here path traversal) can be regarded as suspicious.

- rule: Directory traversal monitored file read
  desc: monitored file read via directory traversal
  condition: open_read and (fd.directory startswith "/etc" or fd.name contains ".ssh/" or fd.name contains "id_rsa") and fd.nameraw contains "../" and fd.nameraw glob *../*../* and not proc.pname in (shell_binaries)
  enabled: true
  output: (username=%user.name useruid=%user.uid user_loginuid=%user.loginuid program=%proc.name exe=%proc.exepath
    command=%proc.cmdline parent=%proc.pname file=%fd.name fileraw=%fd.nameraw parent=%proc.pname
    gparent=%proc.aname[2] container_id=%container.id image=%container.image.repository returncode=%evt.res cwd=%proc.cwd)
  priority: WARNING
  tags: [filesystem, mitre_discovery, mitre_exfiltration]

incertum avatar Jul 14 '22 18:07 incertum

Tweaked it a bit, proposing that it could be even simpler - just look for /etc any file. Also suspecting glob is a more expensive filter, fd.nameraw contains "../" and fd.nameraw glob *../*../* filter may give perf boosts.

# Web applications can be vulnerable to directory traversal attacks that allow 
# accessing files outside of the web app's root directory (e.g. Arbitrary File Read bugs). 
# System directories like /etc are typically accessed via absolute paths, 
# access patterns outside of this (here path traversal) can be regarded as suspicious.

- rule: Directory traversal monitored file read
  desc: monitored file read via directory traversal
  condition: open_read and (fd.directory startswith "/etc" or fd.name contains ".ssh/" or fd.name contains "id_rsa") and fd.nameraw contains "../" and fd.nameraw glob *../*../* and not proc.pname in (shell_binaries)
  enabled: true
  output: (username=%user.name useruid=%user.uid user_loginuid=%user.loginuid program=%proc.name exe=%proc.exepath
    command=%proc.cmdline parent=%proc.pname file=%fd.name fileraw=%fd.nameraw parent=%proc.pname
    gparent=%proc.aname[2] container_id=%container.id image=%container.image.repository returncode=%evt.res cwd=%proc.cwd)
  priority: WARNING
  tags: [filesystem, mitre_discovery, mitre_exfiltration]

Just lookup /etc sounds good to me.

Kaizhe avatar Jul 14 '22 18:07 Kaizhe

Tweaked it a bit, proposing that it could be even simpler - just look for /etc any file. Also suspecting glob is a more expensive filter, fd.nameraw contains "../" and fd.nameraw glob *../*../* filter may give perf boosts.

# Web applications can be vulnerable to directory traversal attacks that allow 
# accessing files outside of the web app's root directory (e.g. Arbitrary File Read bugs). 
# System directories like /etc are typically accessed via absolute paths, 
# access patterns outside of this (here path traversal) can be regarded as suspicious.

- rule: Directory traversal monitored file read
  desc: monitored file read via directory traversal
  condition: open_read and (fd.directory startswith "/etc" or fd.name contains ".ssh/" or fd.name contains "id_rsa") and fd.nameraw contains "../" and fd.nameraw glob *../*../* and not proc.pname in (shell_binaries)
  enabled: true
  output: (username=%user.name useruid=%user.uid user_loginuid=%user.loginuid program=%proc.name exe=%proc.exepath
    command=%proc.cmdline parent=%proc.pname file=%fd.name fileraw=%fd.nameraw parent=%proc.pname
    gparent=%proc.aname[2] container_id=%container.id image=%container.image.repository returncode=%evt.res cwd=%proc.cwd)
  priority: WARNING
  tags: [filesystem, mitre_discovery, mitre_exfiltration]

LGTM @incertum. Amazing job and thenks for all the changes :)

darryk10 avatar Jul 15 '22 06:07 darryk10

LGTM label has been added.

Git tree hash: 0cd3d21c6163d61e25e880f026337225f0ca4430

poiana avatar Jul 21 '22 16:07 poiana

Hey @incertum we just bumped the libs version on master, so now the changes of https://github.com/falcosecurity/libs/pull/468 should be merged into Falco. After rebasing this branch on master, I think tests will pass!

jasondellaluce avatar Aug 03 '22 14:08 jasondellaluce

@darryk10 placed the rule before Read ssh information and used etc_dir macro instead, please re-review, thanks :)

incertum avatar Aug 03 '22 19:08 incertum

(5) Failed file access attempts seem not to be logged -> likely need libs src adjustments for that.

Mystery solved, not a libs issue. Just required some experimentation and the resulting new macro open_file_failed.

In addition, included a minor cleanup (adding parentheses to important macros just to be consistent and safe).

@jasondellaluce @leodido @Kaizhe @darryk10 - Let's brainstorm if the new macro open_file_failed can be of interest for other rules as well? In general, also looking at "failures" or something really out of the norm can help detect suspicious behavior more accurately by making the signal more rich.


Quick Simulation

[removed and not proc.pname in (shell_binaries) from condition for simulation]

{"priority":"Warning","rule":"Directory traversal monitored file read","source":"syscall", "output_fields": {"container.id":"host","container.image.repository":null,"evt.res":"EACCES","fd.name":"/etc/shadow","fd.nameraw":"../../../../../../../etc/shadow","proc.aname[2]":"gnome-terminal-","proc.cmdline":"cat ../../../../../../../etc/shadow","proc.cwd":"/tmp/","proc.exepath":"/usr/bin/cat","proc.name":"cat","proc.pname":"bash","user.loginuid":1000, "user.uid":1000}}
{"priority":"Warning","rule":"Directory traversal monitored file read","source":"syscall", "output_fields": {"container.id":"host","container.image.repository":null,"evt.res":"SUCCESS", "fd.name":"/etc/shadow","fd.nameraw":"../../../../../../../etc/shadow","proc.aname[2]":"sudo","proc.cmdline":"cat ../../../../../../../etc/shadow","proc.cwd":"/tmp/","proc.exepath":"/usr/bin/cat","proc.name":"cat","proc.pname":"sudo","user.loginuid":1000,"user.name":"root","user.uid":0}}

incertum avatar Aug 22 '22 06:08 incertum

LGTM label has been added.

Git tree hash: 2e90fad7b193a1c727a242d5ae5057b6cce962ca

poiana avatar Aug 22 '22 10:08 poiana

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: incertum, jasondellaluce

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

poiana avatar Aug 25 '22 19:08 poiana