mirror of
				https://github.com/neovim/neovim.git
				synced 2025-10-26 12:27:24 +00:00 
			
		
		
		
	vim-patch:9.1.1551: [security]: path traversal issue in zip.vim (#34951)
Problem:  [security]: path traversal issue in zip.vim (@ax)
Solution: drop leading ../ on write of zipfiles, don't forcefully
          overwrite existing files
A zip plugin which contains filenames with leading '../'  may cause
confusion as to where the content will be extracted.  Let's drop such
things and make sure we use a relative filename instead and don't
forcefully overwrite temporary files. Also, warn the user of such
things.
related: vim/vim#17733
586294a041
vim-patch:e1044fb: runtime(zip): raise minimum Vim version to v9.0
vim-patch:e2d9b0d: runtime(zip): zip plugin does not work with Vim 9.0
Co-authored-by: Christian Brabandt <cb@256bit.org>
			
			
This commit is contained in:
		| @@ -15,6 +15,7 @@ | |||||||
| " 2024 Aug 18 by Vim Project: correctly handle special globbing chars | " 2024 Aug 18 by Vim Project: correctly handle special globbing chars | ||||||
| " 2024 Aug 21 by Vim Project: simplify condition to detect MS-Windows | " 2024 Aug 21 by Vim Project: simplify condition to detect MS-Windows | ||||||
| " 2025 Mar 11 by Vim Project: handle filenames with leading '-' correctly | " 2025 Mar 11 by Vim Project: handle filenames with leading '-' correctly | ||||||
|  | " 2025 Jul 12 by Vim Project: drop ../ on write to prevent path traversal attacks | ||||||
| " License:	Vim License  (see vim's :help license) | " License:	Vim License  (see vim's :help license) | ||||||
| " Copyright:	Copyright (C) 2005-2019 Charles E. Campbell {{{1 | " Copyright:	Copyright (C) 2005-2019 Charles E. Campbell {{{1 | ||||||
| "		Permission is hereby granted to use and distribute this code, | "		Permission is hereby granted to use and distribute this code, | ||||||
| @@ -71,8 +72,9 @@ fun! s:Mess(group, msg) | |||||||
|   echohl Normal |   echohl Normal | ||||||
| endfun | endfun | ||||||
|  |  | ||||||
| if v:version < 702 | if !has('nvim-0.10') && v:version < 901 | ||||||
|  call s:Mess('WarningMsg', "***warning*** this version of zip needs vim 7.2 or later") |  " required for defer | ||||||
|  |  call s:Mess('WarningMsg', "***warning*** this version of zip needs vim 9.1 or later") | ||||||
|  finish |  finish | ||||||
| endif | endif | ||||||
| " sanity checks | " sanity checks | ||||||
| @@ -235,59 +237,62 @@ endfun | |||||||
| " zip#Write: {{{2 | " zip#Write: {{{2 | ||||||
| fun! zip#Write(fname) | fun! zip#Write(fname) | ||||||
|   let dict = s:SetSaneOpts() |   let dict = s:SetSaneOpts() | ||||||
|  |   let need_rename = 0 | ||||||
|   defer s:RestoreOpts(dict) |   defer s:RestoreOpts(dict) | ||||||
|  |  | ||||||
|   " sanity checks |   " sanity checks | ||||||
|   if !executable(substitute(g:zip_zipcmd,'\s\+.*$','','')) |   if !executable(substitute(g:zip_zipcmd,'\s\+.*$','','')) | ||||||
|    call s:Mess('Error', "***error*** (zip#Write) sorry, your system doesn't appear to have the ".g:zip_zipcmd." program") |     call s:Mess('Error', "***error*** (zip#Write) sorry, your system doesn't appear to have the ".g:zip_zipcmd." program") | ||||||
|    return |     return | ||||||
|   endif |  | ||||||
|   if !exists("*mkdir") |  | ||||||
|    call s:Mess('Error', "***error*** (zip#Write) sorry, mkdir() doesn't work on your system") |  | ||||||
|    return |  | ||||||
|   endif |   endif | ||||||
|  |  | ||||||
|   let curdir= getcwd() |   let curdir= getcwd() | ||||||
|   let tmpdir= tempname() |   let tmpdir= tempname() | ||||||
|   if tmpdir =~ '\.' |   if tmpdir =~ '\.' | ||||||
|    let tmpdir= substitute(tmpdir,'\.[^.]*$','','e') |     let tmpdir= substitute(tmpdir,'\.[^.]*$','','e') | ||||||
|   endif |   endif | ||||||
|   call mkdir(tmpdir,"p") |   call mkdir(tmpdir,"p") | ||||||
|  |  | ||||||
|   " attempt to change to the indicated directory |   " attempt to change to the indicated directory | ||||||
|   if s:ChgDir(tmpdir,s:ERROR,"(zip#Write) cannot cd to temporary directory") |   if s:ChgDir(tmpdir,s:ERROR,"(zip#Write) cannot cd to temporary directory") | ||||||
|    return |     return | ||||||
|   endif |   endif | ||||||
|  |  | ||||||
|   " place temporary files under .../_ZIPVIM_/ |   " place temporary files under .../_ZIPVIM_/ | ||||||
|   if isdirectory("_ZIPVIM_") |   if isdirectory("_ZIPVIM_") | ||||||
|    call delete("_ZIPVIM_", "rf") |     call delete("_ZIPVIM_", "rf") | ||||||
|   endif |   endif | ||||||
|   call mkdir("_ZIPVIM_") |   call mkdir("_ZIPVIM_") | ||||||
|   cd _ZIPVIM_ |   cd _ZIPVIM_ | ||||||
|  |  | ||||||
|   if has("unix") |   if has("unix") | ||||||
|    let zipfile = substitute(a:fname,'zipfile://\(.\{-}\)::[^\\].*$','\1','') |     let zipfile = substitute(a:fname,'zipfile://\(.\{-}\)::[^\\].*$','\1','') | ||||||
|    let fname   = substitute(a:fname,'zipfile://.\{-}::\([^\\].*\)$','\1','') |     let fname   = substitute(a:fname,'zipfile://.\{-}::\([^\\].*\)$','\1','') | ||||||
|   else |   else | ||||||
|    let zipfile = substitute(a:fname,'^.\{-}zipfile://\(.\{-}\)::[^\\].*$','\1','') |     let zipfile = substitute(a:fname,'^.\{-}zipfile://\(.\{-}\)::[^\\].*$','\1','') | ||||||
|    let fname   = substitute(a:fname,'^.\{-}zipfile://.\{-}::\([^\\].*\)$','\1','') |     let fname   = substitute(a:fname,'^.\{-}zipfile://.\{-}::\([^\\].*\)$','\1','') | ||||||
|  |   endif | ||||||
|  |   if fname =~ '^[.]\{1,2}/' | ||||||
|  |     call system(g:zip_zipcmd." -d ".s:Escape(fnamemodify(zipfile,":p"),0)." ".s:Escape(fname,0)) | ||||||
|  |     let fname = fname->substitute('^\([.]\{1,2}/\)\+', '', 'g') | ||||||
|  |     let need_rename = 1 | ||||||
|   endif |   endif | ||||||
|  |  | ||||||
|   if fname =~ '/' |   if fname =~ '/' | ||||||
|    let dirpath = substitute(fname,'/[^/]\+$','','e') |     let dirpath = substitute(fname,'/[^/]\+$','','e') | ||||||
|    if has("win32unix") && executable("cygpath") |     if has("win32unix") && executable("cygpath") | ||||||
|     let dirpath = substitute(system("cygpath ".s:Escape(dirpath,0)),'\n','','e') |     let dirpath = substitute(system("cygpath ".s:Escape(dirpath,0)),'\n','','e') | ||||||
|    endif |     endif | ||||||
|    call mkdir(dirpath,"p") |     call mkdir(dirpath,"p") | ||||||
|   endif |   endif | ||||||
|   if zipfile !~ '/' |   if zipfile !~ '/' | ||||||
|    let zipfile= curdir.'/'.zipfile |     let zipfile= curdir.'/'.zipfile | ||||||
|   endif |   endif | ||||||
|  |  | ||||||
|   exe "w! ".fnameescape(fname) |   " don't overwrite files forcefully | ||||||
|  |   exe "w ".fnameescape(fname) | ||||||
|   if has("win32unix") && executable("cygpath") |   if has("win32unix") && executable("cygpath") | ||||||
|    let zipfile = substitute(system("cygpath ".s:Escape(zipfile,0)),'\n','','e') |     let zipfile = substitute(system("cygpath ".s:Escape(zipfile,0)),'\n','','e') | ||||||
|   endif |   endif | ||||||
|  |  | ||||||
|   if (has("win32") || has("win95") || has("win64") || has("win16")) && &shell !~? 'sh$' |   if (has("win32") || has("win95") || has("win64") || has("win16")) && &shell !~? 'sh$' | ||||||
| @@ -296,21 +301,24 @@ fun! zip#Write(fname) | |||||||
|  |  | ||||||
|   call system(g:zip_zipcmd." -u ".s:Escape(fnamemodify(zipfile,":p"),0)." ".s:Escape(fname,0)) |   call system(g:zip_zipcmd." -u ".s:Escape(fnamemodify(zipfile,":p"),0)." ".s:Escape(fname,0)) | ||||||
|   if v:shell_error != 0 |   if v:shell_error != 0 | ||||||
|    call s:Mess('Error', "***error*** (zip#Write) sorry, unable to update ".zipfile." with ".fname) |     call s:Mess('Error', "***error*** (zip#Write) sorry, unable to update ".zipfile." with ".fname) | ||||||
|  |  | ||||||
|   elseif s:zipfile_{winnr()} =~ '^\a\+://' |   elseif s:zipfile_{winnr()} =~ '^\a\+://' | ||||||
|    " support writing zipfiles across a network |     " support writing zipfiles across a network | ||||||
|    let netzipfile= s:zipfile_{winnr()} |     let netzipfile= s:zipfile_{winnr()} | ||||||
|    1split|enew |     1split|enew | ||||||
|    let binkeep= &binary |     let binkeep= &binary | ||||||
|    let eikeep = &ei |     let eikeep = &ei | ||||||
|    set binary ei=all |     set binary ei=all | ||||||
|    exe "noswapfile e! ".fnameescape(zipfile) |     exe "noswapfile e! ".fnameescape(zipfile) | ||||||
|    call netrw#NetWrite(netzipfile) |     call netrw#NetWrite(netzipfile) | ||||||
|    let &ei     = eikeep |     let &ei     = eikeep | ||||||
|    let &binary = binkeep |     let &binary = binkeep | ||||||
|    q! |     q! | ||||||
|    unlet s:zipfile_{winnr()} |     unlet s:zipfile_{winnr()} | ||||||
|  |   elseif need_rename | ||||||
|  |     exe $"sil keepalt file {fnameescape($"zipfile://{zipfile}::{fname}")}" | ||||||
|  |     call s:Mess('Warning', "***error*** (zip#Browse) Path Traversal Attack detected, dropping relative path") | ||||||
|   endif |   endif | ||||||
|  |  | ||||||
|   " cleanup and restore current directory |   " cleanup and restore current directory | ||||||
| @@ -319,7 +327,6 @@ fun! zip#Write(fname) | |||||||
|   call s:ChgDir(curdir,s:WARNING,"(zip#Write) unable to return to ".curdir."!") |   call s:ChgDir(curdir,s:WARNING,"(zip#Write) unable to return to ".curdir."!") | ||||||
|   call delete(tmpdir, "rf") |   call delete(tmpdir, "rf") | ||||||
|   setlocal nomod |   setlocal nomod | ||||||
|  |  | ||||||
| endfun | endfun | ||||||
|  |  | ||||||
| " --------------------------------------------------------------------- | " --------------------------------------------------------------------- | ||||||
| @@ -332,15 +339,18 @@ fun! zip#Extract() | |||||||
|  |  | ||||||
|   " sanity check |   " sanity check | ||||||
|   if fname =~ '^"' |   if fname =~ '^"' | ||||||
|    return |     return | ||||||
|   endif |   endif | ||||||
|   if fname =~ '/$' |   if fname =~ '/$' | ||||||
|    call s:Mess('Error', "***error*** (zip#Extract) Please specify a file, not a directory") |     call s:Mess('Error', "***error*** (zip#Extract) Please specify a file, not a directory") | ||||||
|    return |     return | ||||||
|  |   elseif fname =~ '^[.]\?[.]/' | ||||||
|  |     call s:Mess('Error', "***error*** (zip#Browse) Path Traversal Attack detected, not extracting!") | ||||||
|  |     return | ||||||
|   endif |   endif | ||||||
|   if filereadable(fname) |   if filereadable(fname) | ||||||
|    call s:Mess('Error', "***error*** (zip#Extract) <" .. fname .."> already exists in directory, not overwriting!") |     call s:Mess('Error', "***error*** (zip#Extract) <" .. fname .."> already exists in directory, not overwriting!") | ||||||
|    return |     return | ||||||
|   endif |   endif | ||||||
|   let target = fname->substitute('\[', '[[]', 'g') |   let target = fname->substitute('\[', '[[]', 'g') | ||||||
|   " unzip 6.0 does not support -- to denote end-of-arguments |   " unzip 6.0 does not support -- to denote end-of-arguments | ||||||
| @@ -362,13 +372,12 @@ fun! zip#Extract() | |||||||
|   " extract the file mentioned under the cursor |   " extract the file mentioned under the cursor | ||||||
|   call system($"{g:zip_extractcmd} -o {shellescape(b:zipfile)} {target}") |   call system($"{g:zip_extractcmd} -o {shellescape(b:zipfile)} {target}") | ||||||
|   if v:shell_error != 0 |   if v:shell_error != 0 | ||||||
|    call s:Mess('Error', "***error*** ".g:zip_extractcmd." ".b:zipfile." ".fname.": failed!") |     call s:Mess('Error', "***error*** ".g:zip_extractcmd." ".b:zipfile." ".fname.": failed!") | ||||||
|   elseif !filereadable(fname) |   elseif !filereadable(fname) | ||||||
|    call s:Mess('Error', "***error*** attempted to extract ".fname." but it doesn't appear to be present!") |     call s:Mess('Error', "***error*** attempted to extract ".fname." but it doesn't appear to be present!") | ||||||
|   else |   else | ||||||
|    echomsg "***note*** successfully extracted ".fname |     echomsg "***note*** successfully extracted ".fname | ||||||
|   endif |   endif | ||||||
|  |  | ||||||
| endfun | endfun | ||||||
|  |  | ||||||
| " --------------------------------------------------------------------- | " --------------------------------------------------------------------- | ||||||
|   | |||||||
| @@ -111,6 +111,18 @@ Copyright: Copyright (C) 2005-2015 Charles E Campbell	 *zip-copyright* | |||||||
|  |  | ||||||
| ============================================================================== | ============================================================================== | ||||||
| 4. History							*zip-history* {{{1 | 4. History							*zip-history* {{{1 | ||||||
|  |    unreleased: | ||||||
|  |        Jul 12, 2025 * drop ../ on write to prevent path traversal attacks | ||||||
|  |        Mar 11, 2025 * handle filenames with leading '-' correctly | ||||||
|  |        Aug 21, 2024 * simplify condition to detect MS-Windows | ||||||
|  |        Aug 18, 2024 * correctly handle special globbing chars | ||||||
|  |        Aug 05, 2024 * clean-up and make it work with shellslash on Windows | ||||||
|  |        Aug 05, 2024 * workaround for the FreeBSD's unzip | ||||||
|  |        Aug 04, 2024 * escape '[' in name of file to be extracted | ||||||
|  |        Jul 30, 2024 * fix opening remote zipfile | ||||||
|  |        Jul 24, 2024 * use delete() function | ||||||
|  |        Jul 23, 2024 * fix 'x' command | ||||||
|  |        Jun 16, 2024 * handle whitespace on Windows properly (#14998) | ||||||
|    v33 Dec 07, 2021 * `*.xlam` mentioned twice in zipPlugin |    v33 Dec 07, 2021 * `*.xlam` mentioned twice in zipPlugin | ||||||
|    v32 Oct 22, 2021 * to avoid an issue with a vim 8.2 patch, zipfile: has |    v32 Oct 22, 2021 * to avoid an issue with a vim 8.2 patch, zipfile: has | ||||||
| 		      been changed to zipfile:// . This often shows up | 		      been changed to zipfile:// . This often shows up | ||||||
|   | |||||||
							
								
								
									
										
											BIN
										
									
								
								test/old/testdir/samples/evil.zip
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										
											BIN
										
									
								
								test/old/testdir/samples/evil.zip
									
									
									
									
									
										Normal file
									
								
							
										
											Binary file not shown.
										
									
								
							| @@ -9,13 +9,14 @@ endif | |||||||
|  |  | ||||||
| runtime plugin/zipPlugin.vim | runtime plugin/zipPlugin.vim | ||||||
|  |  | ||||||
| func Test_zip_basic() | func s:CopyZipFile(source) | ||||||
|  |   if !filecopy($"samples/{a:source}", "X.zip") | ||||||
|   "## get our zip file |     call assert_report($"Can't copy samples/{a:source}.zip") | ||||||
|   if !filecopy("samples/test.zip", "X.zip") |  | ||||||
|     call assert_report("Can't copy samples/test.zip") |  | ||||||
|     return |  | ||||||
|   endif |   endif | ||||||
|  | endfunc | ||||||
|  |  | ||||||
|  | func Test_zip_basic() | ||||||
|  |   call s:CopyZipFile("test.zip") | ||||||
|   defer delete("X.zip") |   defer delete("X.zip") | ||||||
|  |  | ||||||
|   e X.zip |   e X.zip | ||||||
| @@ -142,11 +143,7 @@ func Test_zip_glob_fname() | |||||||
|   CheckNotMSWindows |   CheckNotMSWindows | ||||||
|   " does not work on Windows, why? |   " does not work on Windows, why? | ||||||
|  |  | ||||||
|   "## copy sample zip file |   call s:CopyZipFile("testa.zip") | ||||||
|   if !filecopy("samples/testa.zip", "X.zip") |  | ||||||
|     call assert_report("Can't copy samples/testa.zip") |  | ||||||
|     return |  | ||||||
|   endif |  | ||||||
|   defer delete("X.zip") |   defer delete("X.zip") | ||||||
|   defer delete('zipglob', 'rf') |   defer delete('zipglob', 'rf') | ||||||
|  |  | ||||||
| @@ -240,10 +237,7 @@ func Test_zip_fname_leading_hyphen() | |||||||
|   CheckNotMSWindows |   CheckNotMSWindows | ||||||
|  |  | ||||||
|   "## copy sample zip file |   "## copy sample zip file | ||||||
|   if !filecopy("samples/poc.zip", "X.zip") |   call s:CopyZipFile("poc.zip") | ||||||
|     call assert_report("Can't copy samples/poc.zip") |  | ||||||
|     return |  | ||||||
|   endif |  | ||||||
|   defer delete("X.zip") |   defer delete("X.zip") | ||||||
|   defer delete('-d', 'rf') |   defer delete('-d', 'rf') | ||||||
|   defer delete('/tmp/pwned', 'rf') |   defer delete('/tmp/pwned', 'rf') | ||||||
| @@ -258,3 +252,26 @@ func Test_zip_fname_leading_hyphen() | |||||||
|   call assert_false(filereadable('/tmp/pwned')) |   call assert_false(filereadable('/tmp/pwned')) | ||||||
|   bw |   bw | ||||||
| endfunc | endfunc | ||||||
|  |  | ||||||
|  | func Test_zip_fname_evil_path() | ||||||
|  |   CheckNotMSWindows | ||||||
|  |   " needed for writing the zip file | ||||||
|  |   CheckExecutable zip | ||||||
|  |  | ||||||
|  |   call s:CopyZipFile("evil.zip") | ||||||
|  |   defer delete("X.zip") | ||||||
|  |   e X.zip | ||||||
|  |  | ||||||
|  |   :1 | ||||||
|  |   let fname = 'pwn' | ||||||
|  |   call search('\V' .. fname) | ||||||
|  |   normal x | ||||||
|  |   call assert_false(filereadable('/etc/ax-pwn')) | ||||||
|  |   let mess  = execute(':mess') | ||||||
|  |   call assert_match('Path Traversal Attack', mess) | ||||||
|  |  | ||||||
|  |   exe ":normal \<cr>" | ||||||
|  |   :w | ||||||
|  |   call assert_match('zipfile://.*::etc/ax-pwn', @%) | ||||||
|  |   bw | ||||||
|  | endfunc | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 zeertzjq
					zeertzjq