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

Use https to alleviate potential mitm vulnerability #564

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gmarik
Copy link
Contributor

@gmarik gmarik commented Feb 27, 2015

I've been contacted to address this issue, and quick fix would be using https protocol instead of http.

It's the same file just using straight from repo instead going through vim-scripts domain.

UNTESTED.

@gmarik gmarik added the bug label Feb 27, 2015
@jdevera
Copy link
Contributor

jdevera commented Feb 28, 2015

I think in some installations of curl this will stop working because of the http -> https switch, but I guess we can alleviate the effect with documentation.

@gmarik
Copy link
Contributor Author

gmarik commented Feb 28, 2015

I think in some installations of curl this will stop working because of the http -> https switch

Should url be configurable then?

@jdevera
Copy link
Contributor

jdevera commented Mar 2, 2015

We could infer it. The output of curl -V contains SSL when https is supported.

Alternatively, we can make it configurable, but not the url itself, but rather to use https or http, and of course having https as default.

What do you think? Infer or configure?

@gmarik
Copy link
Contributor Author

gmarik commented Mar 2, 2015

it should be https by default and whenever it fails it needs to suggest to try http one with warning of possible vulnerability.

To much ceremony you think?

@gmarik
Copy link
Contributor Author

gmarik commented Mar 2, 2015

Alternatively we could abandon JSON altogether and just go with plaintext file: 1) no vulnerability 2) faster

@jdevera
Copy link
Contributor

jdevera commented Mar 21, 2015

I like that option even better, one plugin name per line, we use readfile(). Simple.

@ecneladis
Copy link

+1, critical

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

Successfully merging this pull request may close these issues.

None yet

3 participants