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

Add security policy file / documentation #1377

Merged
merged 1 commit into from
Mar 4, 2023

Conversation

ychin
Copy link
Member

@ychin ychin commented Mar 4, 2023

GitHub has a security tab that allows repos to manage their security policy so it's not a bad idea ot be explicit in expectations. The policy is to either use GitHub's builtin reporting system, or email MacVim's team (in case that's the preferred method or the reporter does not want to have a GitHub account). The most important thing is to not use the public GitHub issue filing.

I don't think this will be used too much, but given that MacVim (and Vim) can read arbitrary file, there is always a potential for a security issue to pop up.

GitHub has a security tab that allows repos to manage their security
policy so it's not a bad idea ot be explicit in expectations. The policy
is to either use GitHub's builtin reporting system, or email MacVim's
team (in case that's the preferred method or the reporter does not want
to have a GitHub account). The most important thing is to not use the
public GitHub issue filing.

I don't think this will be used too much, but given that MacVim (and
Vim) can read arbitrary file, there is always a potential for a security
issue to pop up.
@eirnym
Copy link
Contributor

eirnym commented Mar 4, 2023

It's a very good idea!

given that MacVim (and Vim) can read arbitrary file

If a user explicitly allows that (if macOS is new enough)

@ychin
Copy link
Member Author

ychin commented Mar 4, 2023

That's true, but I really meant it in a more general sense. When I download a file from the internet, I may be hesitant to run an executable, but most people probably would feel fine opening it in a text editor. In fact, a lot of times I probably open a suspicious file in Vim first just to make sure it looks kosher, and Vim has been subject to vulnerabilities before…

Well hopefully the tab will not get used much.

@eirnym
Copy link
Contributor

eirnym commented Mar 4, 2023

Correct, this was just an important note on the topic.

Modeline was a well known topic for Vim itself. Assuming this vector is excluded, I personally believe that Vim part is safe enough to open an arbitrary file.

So I see following main basic attack vectors for MacVim without diving too much into the code:

  • font rendering
  • terminal emulator
  • MacVim-related changes in Vim core

Plugins-related part… I have a mind that any third-party plugin is vulnerable, so it's important to audit it before download it.

@eirnym
Copy link
Contributor

eirnym commented Mar 4, 2023

With that in mind, I suggest to add a code analysis tools using GitHub Workflows. To know ahead if a commit introduces a potential vulnerability.

What do you think, @ychin?

@ychin
Copy link
Member Author

ychin commented Mar 4, 2023

What kind of code analysis tool do you have in mind? They could be useful but different tools have different strengths and we want to avoid overly verbose false positives that would end up wasting our time. Also, Objective C is not frequently covered (e.g. it's not supported by CodeQL).

@ychin ychin merged commit df0ffe9 into macvim-dev:master Mar 4, 2023
@ychin ychin deleted the add-security-policy branch March 4, 2023 11:04
@eirnym
Copy link
Contributor

eirnym commented Mar 4, 2023

I found quite a few tools to use:

@eirnym
Copy link
Contributor

eirnym commented Mar 4, 2023

And obviously, there's also clang, which has it's own quite powerful static analysis tool

@ychin
Copy link
Member Author

ychin commented Mar 13, 2023

I may eventually take a look at some of them. Feel free to file a separate issue if you want since I'm not going to remember to look back at this thread.

FWIW anything proprietary is out of the question for me. I tried to install Infer through Homebrew (their recommended method) and it's basically broken and then I gave up.

Also, currently the code is still not warning free even though I have fixed a few of them already. Getting that to be able to compile without warnings is probably the most important. But using clang's ASAN etc tools later on would be useful.

@ychin ychin added the Non User Facing Non-user facing change. These issues do no need to show up in release notes. label Mar 20, 2023
tono pushed a commit to tono/macvim that referenced this pull request Mar 21, 2023
Add security policy file / documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Non User Facing Non-user facing change. These issues do no need to show up in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants