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

How to protect our metadata fields from code injection attacks? #28

Closed
joshgoebel opened this issue Jul 3, 2022 · 3 comments
Closed
Labels
question Further information is requested security Security related

Comments

@joshgoebel
Copy link
Contributor

joshgoebel commented Jul 3, 2022

I just realized (in talking about security in the other thread) that our metadata fields are likely currently subject to code injection attacks via a malicious theme.

From vim template:

" base16-vim (https://github.com/chriskempson/base16-vim)
" by Chris Kempson (http://chriskempson.com)
" {{scheme-name}} scheme by {{scheme-author}}

Example of the attack vector:

(this is using a YAML multi-line string)

scheme: |
  trojan-horsey-unicorn

  [here is a huge
  glob of evil and
  malicious vimscript]
base00: "090300" #  ----

I'd guess this is exploitable for any templates where the output is an executable file... we could start by not allowing multiple lines (perhaps make it an error case) but how would you handle protecting all the possible templates since we don't know which characters might be used to escape commenting?

The assumption of course being that these metadata fields are only for use in comments... and should NOT be allowed to escape those comments.

@joshgoebel joshgoebel added help wanted Extra attention is needed question Further information is requested security Security related labels Jul 3, 2022
@belak
Copy link
Member

belak commented Jul 3, 2022

I think this is primarily a policy issue - we control the schemes repo, so we'll just have to watch for it.

The easiest way to handle it would be to limit what characters are allowed in color scheme fields. Only allow text and general sentence punctuation, no newlines (we could optionally require replacing newlines with spaces in the builder spec).

@belak belak changed the title Security: How to protect our metadata fields from code injection attacks? How to protect our metadata fields from code injection attacks? Jul 6, 2022
@belak belak removed the help wanted Extra attention is needed label Jul 8, 2022
@belak
Copy link
Member

belak commented Jul 12, 2022

I'm closing this for now, even though we don't have a great answer - at the moment, we're just going to rely on making sure we don't provide any malformed schemes in any of the repos/systems we support. That means keeping values, even description, to a single line.

@belak belak closed this as completed Jul 12, 2022
@joshgoebel
Copy link
Contributor Author

Is there harm in leaving an issue open when it is both:

  1. unresolved
  2. a known potential security issue

? I would have though that #1 was enough (on it's own) but the pair seems doubly important... Isn't this why we have tags like "help wanted"?

I know you like to move fast, but some issues take time to resolve... the issue doesn't go away because it's closed. If someone wants to come along and has an idea to help resolve it, I wouldn't want to miss that help because this was prematurely closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested security Security related
Projects
None yet
Development

No branches or pull requests

2 participants