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

Disallow registering interpreters for miscellaneous binary formats #249

Merged
merged 7 commits into from
Aug 16, 2024

Conversation

raja-grewal
Copy link
Contributor

Disallow registering interpreters for various (miscellaneous) binary formats based on a magic number or their file extension to prevent unintended code execution.

Breaks many scripts that do not have appropriate shebang interpreter directives (#!/bin/...).

Changes

Set sysctl fs.binfmt_misc.status=0.

Mandatory Checklist

  • Legal agreements accepted. By contributing to this organisation, you acknowledge you have read, understood, and agree to be bound by these these agreements:

Terms of Service, Privacy Policy, Cookie Policy, E-Sign Consent, DMCA, Imprint

Optional Checklist

The following items are optional but might be requested in certain cases.

  • I have tested it locally
  • I have reviewed and updated any documentation if relevant
  • I am providing new code and test(s) for it

@adrelanos
Copy link
Member

But for execution, the execution bit ("chmod +x") is still required? No execution bit -> no binfmt_misc -> no security issue?

The execution bit afaik cannot set itself.

Also trivial to circumvent for an attacker. Instead of only using .sh the attack would keep using .sh but also add a proper shebang? So this, while interesting, I don't see what advantage it provides?

@raja-grewal
Copy link
Contributor Author

Great points, you are correct.

I suppose the only scenario in which this parameter is useful is the case of the less sophisticated scripts after being granted the execution bit.

It might still be useful perhaps against older existing legacy malware that has yet to adapt?

So suppose user grants the execution bit and the script is actually invoking other interpreter without user being aware.

There are at least 3 ways to proceed with this PR:

  1. Close it since it is likely not that effective against modern attack vectors.
  2. Comment out the sysctl but leave the documentation there for future reference with a link to this PR.
  3. Enable it as way to mitigate against legacy scripts that may be intentionally doing undesirable things.

How would you like to proceed?

@adrelanos
Copy link
Member

It might still be useful perhaps against older existing legacy malware that has yet to adapt?

Not sure there is any legacy malware where this would have been the case?

1. Close it since it is likely not that effective against modern attack vectors.

Yes.

2. Comment out the `sysctl` but leave the documentation there for future reference with a link to this PR.

This seems useful.

@raja-grewal
Copy link
Contributor Author

Done. I have gone with option 2 and made the sysctl optional by commenting out the command but leaving the documentation intact.

@raja-grewal
Copy link
Contributor Author

Note KSPP also recommends setting fs.binfmt_misc.status=0 via 'equivalent' kernel CONFIGs:

# Easily confused by misconfigured userspace, keep off.
# CONFIG_BINFMT_MISC is not set

So should we adhere to KSPP?

@adrelanos
Copy link
Member

Yes. Could you please document with this reasoning?

@raja-grewal
Copy link
Contributor Author

Done.

The KSPP notice style is preliminary and we can change it later to be consistent with what will be eventually decided as per #256.

@adrelanos
Copy link
Member

Looks great!

Where can we see that fs.binfmt_misc.status is a valid kernel setting? I cannot find it on kernel.org. Search term:

fs.binfmt_misc.status site:kernel .org

Cannot find a manual for it.

sudo sysctl -a | grep fs.binfmt_misc.status

fs.binfmt_misc.status = enabled

That would imply that we want to set fs.binfmt_misc.status=disabled.

How to actually test if the setting change takes effect?

@raja-grewal
Copy link
Contributor Author

I am not sure why it does not show up properly on kernel.org here:
https://www.kernel.org/doc/html/latest/admin-guide/sysctl/fs.html
https://www.kernel.org/doc/html/latest/admin-guide/binfmt-misc.html

I think it may just be case of the documentation not having been updated?

Regarding functionality, as per the references, first see here:
https://unix.stackexchange.com/questions/439569/what-kinds-of-executable-formats-do-the-files-under-proc-sys-fs-binfmt-misc-al

By default:

[a@X] doas sysctl fs.binfmt_misc.status
fs.binfmt_misc.status = enabled

[a@X] doas ls -l /proc/sys/fs/binfmt_misc/
total 0
--w------- 1 root root 0 Aug 16 11:37 register
-rw-r--r-- 1 root root 0 Aug 16 12:21 status

Next, trying your suggestion:

[a@X] doas sysctl fs.binfmt_misc.status=disabled
sysctl: setting key "fs.binfmt_misc.status": Invalid argument

By this PR:

[a@X] doas sysctl fs.binfmt_misc.status=0
fs.binfmt_misc.status = 0

[a@X] doas sysctl fs.binfmt_misc.status
fs.binfmt_misc.status = disabled

[a@X] doas ls -l /proc/sys/fs/binfmt_misc/
total 0
--w------- 1 root root 0 Aug 16 11:37 register
-rw-r--r-- 1 root root 0 Aug 16 12:26 status

It appears that kernel only recognises =0 to disable.

My system is rather hardened and so I do not have many executable formats so can not display any changes on my end.

To then test that it is functioning properly, one can always use doas sysctl fs.binfmt_misc.status to ensure that the functionality is disabled.

@adrelanos
Copy link
Member

Alright, sudo sysctl -w fs.binfmt_misc.status=1 vs 0 switches from enabled to disabled when checking using sudo sysctl fs.binfmt_misc.status so this seems correct indeed.

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

Successfully merging this pull request may close these issues.

2 participants