From: Christian Brabandt Date: Thu, 4 Jun 2026 21:06:09 +0000 (+0000) Subject: patch 9.2.0597: [security]: possible code execution with python complete X-Git-Tag: v9.2.0597^0 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=c8c63673bc4253212820626aeeb75999d9a539d2;p=thirdparty%2Fvim.git patch 9.2.0597: [security]: possible code execution with python complete Problem: [security]: another possible code execution with python complete (David Carliez) Solution: Strip default expressions and annotations from generated source for pythoncomplete and python3complete. Github Security Advisory: https://github.com/vim/vim/security/advisories/GHSA-65p9-mwwx-7468 Signed-off-by: Christian Brabandt --- diff --git a/runtime/autoload/python3complete.vim b/runtime/autoload/python3complete.vim index 0cbfc02ab9..4b6f5d2ced 100644 --- a/runtime/autoload/python3complete.vim +++ b/runtime/autoload/python3complete.vim @@ -1,8 +1,8 @@ "python3complete.vim - Omni Completion for python " Maintainer: " Previous Maintainer: Aaron Griffin -" Version: 0.9 -" Last Updated: 2022 Mar 30 +" Version: 0.10 +" Last Updated: 2026 Jun 04 " " Roland Puntaier: this file contains adaptations for python3 and is parallel to pythoncomplete.vim " @@ -17,6 +17,11 @@ " v 0.10 by Vim project " * disables importing local modules, unless the global Vim variable " g:pythoncomplete_allow_import is set to non-zero +" * strip default values and annotations from function parameter lists +" before exec(), and whitelist class base lists to dotted names: the +" previous code passed buffer-supplied expressions to exec() which +" Python evaluates at definition time, allowing arbitrary code +" execution via crafted def/class headers " " v 0.9 " * Fixed docstring parsing for classes and functions @@ -100,6 +105,24 @@ warnings.simplefilter(action='ignore', category=FutureWarning) import sys, tokenize, io, types from token import NAME, DEDENT, NEWLINE, STRING +import re + +# Used by Class.get_code(): a base class expression is only included in the +# code passed to exec() if it is a pure dotted name (e.g. "Base", "mod.Base", +# "pkg.sub.Cls"). Anything containing calls, subscripts, "=", ":" or other +# operators is dropped, since exec()-ing it would evaluate buffer-supplied +# expressions. See the security note in the file header. +_DOTTED_NAME_RE = re.compile(r'^[A-Za-z_]\w*(\s*\.\s*[A-Za-z_]\w*)*$') + +def _strip_param(p): + # Return the bare parameter name from a parameter spec harvested by + # _parenparse(), discarding any default value or annotation. Default + # values and annotations would otherwise be evaluated by exec() at + # function-definition time. Star prefixes ("*args", "**kw") and bare + # "*" / "/" are preserved as written. + p = p.split('=', 1)[0] + p = p.split(':', 1)[0] + return p.strip() debugstmts=[] def dbg(s): debugstmts.append(s) @@ -350,7 +373,13 @@ class Class(Scope): return c def get_code(self): str = '%sclass %s' % (self.currentindent(),self.name) - if len(self.supers) > 0: str += '(%s)' % ','.join(self.supers) + # Only include base class expressions that are pure dotted names. + # Anything else (calls, subscripts, conditionals, ...) is dropped + # because exec() would evaluate it at class-definition time. See + # the security note in the file header. + safe_supers = [s.strip() for s in self.supers + if _DOTTED_NAME_RE.match(s.strip())] + if len(safe_supers) > 0: str += '(%s)' % ','.join(safe_supers) str += ':\n' if len(self.docstr) > 0: str += self.childindent()+'"""'+self.docstr+'"""\n' if len(self.subscopes) > 0: @@ -367,8 +396,14 @@ class Function(Scope): def copy_decl(self,indent=0): return Function(self.name,self.params,indent, self.docstr) def get_code(self): + # Strip default values and annotations from each parameter before + # joining: exec() evaluates these at definition time and a hostile + # buffer could otherwise execute arbitrary code via crafted def + # headers. See file header for details. + safe_params = [_strip_param(p) for p in self.params] + safe_params = [p for p in safe_params if p] str = "%sdef %s(%s):\n" % \ - (self.currentindent(),self.name,','.join(self.params)) + (self.currentindent(),self.name,','.join(safe_params)) if len(self.docstr) > 0: str += self.childindent()+'"""'+self.docstr+'"""\n' str += "%spass\n" % self.childindent() return str diff --git a/runtime/autoload/pythoncomplete.vim b/runtime/autoload/pythoncomplete.vim index b4340f7ae2..7614882446 100644 --- a/runtime/autoload/pythoncomplete.vim +++ b/runtime/autoload/pythoncomplete.vim @@ -1,8 +1,8 @@ "pythoncomplete.vim - Omni Completion for python " Maintainer: " Previous Maintainer: Aaron Griffin -" Version: 0.9 -" Last Updated: 2020 Oct 9 +" Version: 0.10 +" Last Updated: 2026 Jun 04 " " Changes " TODO: @@ -15,6 +15,11 @@ " v 0.10 by Vim project " * disables importing local modules, unless the global Vim variable " g:pythoncomplete_allow_import is set to non-zero +" * strip default values and annotations from function parameter lists +" before exec(), and whitelist class base lists to dotted names: the +" previous code passed buffer-supplied expressions to exec() which +" Python evaluates at definition time, allowing arbitrary code +" execution via crafted def/class headers " " v 0.9 " * Fixed docstring parsing for classes and functions @@ -95,6 +100,24 @@ function! s:DefPython() python << PYTHONEOF import sys, tokenize, cStringIO, types from token import NAME, DEDENT, NEWLINE, STRING +import re + +# Used by Class.get_code(): a base class expression is only included in the +# code passed to exec() if it is a pure dotted name (e.g. "Base", "mod.Base", +# "pkg.sub.Cls"). Anything containing calls, subscripts, "=", ":" or other +# operators is dropped, since exec()-ing it would evaluate buffer-supplied +# expressions. See the security note in the file header. +_DOTTED_NAME_RE = re.compile(r'^[A-Za-z_]\w*(\s*\.\s*[A-Za-z_]\w*)*$') + +def _strip_param(p): + # Return the bare parameter name from a parameter spec harvested by + # _parenparse(), discarding any default value or annotation. Default + # values and annotations would otherwise be evaluated by exec() at + # function-definition time. Star prefixes ("*args", "**kw") and bare + # "*" / "/" are preserved as written. + p = p.split('=', 1)[0] + p = p.split(':', 1)[0] + return p.strip() debugstmts=[] def dbg(s): debugstmts.append(s) @@ -365,7 +388,13 @@ class Class(Scope): return c def get_code(self): str = '%sclass %s' % (self.currentindent(),self.name) - if len(self.supers) > 0: str += '(%s)' % ','.join(self.supers) + # Only include base class expressions that are pure dotted names. + # Anything else (calls, subscripts, conditionals, ...) is dropped + # because exec() would evaluate it at class-definition time. See + # the security note in the file header. + safe_supers = [s.strip() for s in self.supers + if _DOTTED_NAME_RE.match(s.strip())] + if len(safe_supers) > 0: str += '(%s)' % ','.join(safe_supers) str += ':\n' if len(self.docstr) > 0: str += self.childindent()+'"""'+self.docstr+'"""\n' if len(self.subscopes) > 0: @@ -382,8 +411,14 @@ class Function(Scope): def copy_decl(self,indent=0): return Function(self.name,self.params,indent, self.docstr) def get_code(self): + # Strip default values and annotations from each parameter before + # joining: exec() evaluates these at definition time and a hostile + # buffer could otherwise execute arbitrary code via crafted def + # headers. See file header for details. + safe_params = [_strip_param(p) for p in self.params] + safe_params = [p for p in safe_params if p] str = "%sdef %s(%s):\n" % \ - (self.currentindent(),self.name,','.join(self.params)) + (self.currentindent(),self.name,','.join(safe_params)) if len(self.docstr) > 0: str += self.childindent()+'"""'+self.docstr+'"""\n' str += "%spass\n" % self.childindent() return str diff --git a/src/testdir/Make_all.mak b/src/testdir/Make_all.mak index 537c5250f7..b81e7e6bf6 100644 --- a/src/testdir/Make_all.mak +++ b/src/testdir/Make_all.mak @@ -251,6 +251,7 @@ NEW_TESTS = \ test_plugin_matchit \ test_plugin_matchparen \ test_plugin_netrw \ + test_plugin_python3complete \ test_plugin_osc52 \ test_plugin_tar \ test_plugin_termdebug \ @@ -530,6 +531,7 @@ NEW_TESTS_RES = \ test_plugin_matchit.res \ test_plugin_matchparen.res \ test_plugin_netrw.res \ + test_plugin_python3complete.res \ test_plugin_osc52.res \ test_plugin_tar.res \ test_plugin_termdebug.res \ diff --git a/src/testdir/test_plugin_python3complete.vim b/src/testdir/test_plugin_python3complete.vim new file mode 100644 index 0000000000..e2b0c6616d --- /dev/null +++ b/src/testdir/test_plugin_python3complete.vim @@ -0,0 +1,224 @@ +" Tests for the Python omni-completion plugin (runtime/autoload/python3complete.vim). +" +CheckFeature python3 + +" Run omni-completion against the given buffer contents and assert that the +" marker file was not created. Pre-patch behaviour exec()s reconstructed +" def/class headers, which evaluates the buffer-supplied expression and +" creates the marker file. Post-patch, the expressions are stripped. +func s:CompleteAndExpectNoMarker(buffer_lines, marker_path, msg) + call delete(a:marker_path) + defer delete(a:marker_path) + let g:pythoncomplete_allow_import = 0 + new + setfiletype python + call setline(1, a:buffer_lines) + call cursor(line('$'), col([line('$'), '$'])) + + " The PoC trigger -- direct invocation of the omnifunc with an empty base. + " This is the same path Vim takes for CTRL-X CTRL-O. + silent! call python3complete#Complete(0, '') + + call assert_false(filereadable(a:marker_path), + \ a:msg . ' (marker ' . a:marker_path . ' was created)') + + bwipe! + unlet! g:pythoncomplete_allow_import +endfunc + +func Test_python3complete_no_exec_via_function_default() + let marker = tempname() + call s:CompleteAndExpectNoMarker([ + \ 'def f(x=open(' . string(marker) . ', "w").close()):', + \ ' pass', + \ 'f.', + \ ], marker, + \ 'function default expression was evaluated during omni-completion') +endfunc + +func Test_python3complete_no_exec_via_function_annotation() + let marker = tempname() + call s:CompleteAndExpectNoMarker([ + \ 'def f(x: open(' . string(marker) . ', "w").close()):', + \ ' pass', + \ 'f.', + \ ], marker, + \ 'function annotation expression was evaluated during omni-completion') +endfunc + +func Test_python3complete_no_exec_via_class_base() + let marker = tempname() + " "or object" gives the class a valid base after the side-effecting + " open().close() expression returns None. Without "or object" the + " exec would raise TypeError, but the file would still be created + " before the exception -- the assertion would still hold. Using + " "or object" keeps the buffer parseable as valid Python. + call s:CompleteAndExpectNoMarker([ + \ 'class Foo(open(' . string(marker) . ', "w").close() or object):', + \ ' pass', + \ 'Foo.', + \ ], marker, + \ 'class base expression was evaluated during omni-completion') +endfunc + +func Test_python3complete_no_exec_with_multiple_params() + " The strip must apply to every parameter, not just the first. + let marker = tempname() + call s:CompleteAndExpectNoMarker([ + \ 'def f(a, b=1, c=open(' . string(marker) . ', "w").close(), d=2):', + \ ' pass', + \ 'f.', + \ ], marker, + \ 'non-first parameter default was evaluated during omni-completion') +endfunc + +func Test_python3complete_no_exec_via_starargs_default() + " "*args" and "**kw" must still be preserved after stripping; ensure a + " default following them is also stripped. + let marker = tempname() + call s:CompleteAndExpectNoMarker([ + \ 'def f(*args, key=open(' . string(marker) . ', "w").close(), **kw):', + \ ' pass', + \ 'f.', + \ ], marker, + \ 'keyword-only default after *args was evaluated during omni-completion') +endfunc + +func Test_python3complete_normal_completion_still_works() + " Positive control: completion against a buffer with a legitimate class + " must still produce completion items. The stripping logic should not + " break the normal completion path. + let g:pythoncomplete_allow_import = 0 + + new + setfiletype python + call setline(1, [ + \ 'class MyHelper:', + \ ' def alpha(self): pass', + \ ' def beta(self): pass', + \ 'h = MyHelper()', + \ 'h.', + \ ]) + call cursor(5, 3) + + " First call returns the column to start completion at; second returns + " the list of completion items. + let start = python3complete#Complete(1, '') + call assert_true(start >= 0, + \ 'python3complete#Complete(1, "") returned ' . start) + + let items = python3complete#Complete(0, '') + " Items should be a list (possibly empty if the parser can't resolve "h", + " but should not be a parse error from our stripping changes). + call assert_equal(type([]), type(items), + \ 'python3complete#Complete(0, "") did not return a list') + + bwipe! + unlet! g:pythoncomplete_allow_import +endfunc + +func Test_python3complete_inherited_completion_via_dotted_base() + " Positive control for the class-base whitelist: a dotted-name base class + " (the common, safe case) must still be carried into the reconstructed + " source so that completion on a subclass can resolve inherited members. + let g:pythoncomplete_allow_import = 0 + + new + setfiletype python + call setline(1, [ + \ 'class Base:', + \ ' def shared(self): pass', + \ 'class Derived(Base):', + \ ' def own(self): pass', + \ 'd = Derived()', + \ 'd.', + \ ]) + call cursor(6, 3) + + let items = python3complete#Complete(0, '') + call assert_equal(type([]), type(items), + \ 'completion against a subclass with a dotted base did not return a list') + + bwipe! + unlet! g:pythoncomplete_allow_import +endfunc + +" Build a tiny Python module that creates a marker file as a side effect of +" being imported, add its directory to sys.path, run omni-completion against +" a buffer containing `import vimtest_marker_mod`, and report whether the +" marker file was created. Used by the two allow_import tests below. +func s:RunImportCompletion(allow_import_value) + let g:pythoncomplete_allow_import = a:allow_import_value + let marker = tempname() + let module_dir = tempname() + call mkdir(module_dir, 'R') + + call writefile([ + \ 'open(' . string(marker) . ', "w").close()', + \ ], module_dir . '/vimtest_marker_mod.py') + + defer delete(marker) + + " Pass module_dir to Python via a g: variable so vim.eval() can read it. + let g:pythoncomplete_test_module_dir = module_dir + py3 << EOF +import sys, vim +_p = vim.eval('g:pythoncomplete_test_module_dir') +if _p not in sys.path: + sys.path.insert(0, _p) +# Drop any cached copy so the module body re-runs and the marker side +# effect fires on import. +sys.modules.pop('vimtest_marker_mod', None) +EOF + + new + setfiletype python + call setline(1, [ + \ 'import vimtest_marker_mod', + \ 'vimtest_marker_mod.', + \ ]) + call cursor(2, 2) + + silent! call python3complete#Complete(0, '') + + let ran = filereadable(marker) + + bwipe! + unlet g:pythoncomplete_allow_import + + " Teardown: restore sys.path, drop the cached module so a subsequent + " test run starts clean, clean up the temp module dir. + py3 << EOF +import sys, vim +_p = vim.eval('g:pythoncomplete_test_module_dir') +if _p in sys.path: + sys.path.remove(_p) +sys.modules.pop('vimtest_marker_mod', None) +EOF + unlet g:pythoncomplete_test_module_dir + call delete(module_dir, 'rf') + call delete(marker) + unlet! g:pythoncomplete_allow_import + + return ran +endfunc + +func Test_python3complete_allow_import_off_blocks_imports() + " GHSA-52mc-rq6p-rc7c mitigation: with the default flag value (0), an + " `import` line harvested from the buffer must NOT be exec()'d. The + " marker module's side effect (creating a file when its body runs) is + " the observable proof that the exec did or did not happen. + call assert_false(s:RunImportCompletion(0), + \ 'g:pythoncomplete_allow_import=0 did not block the buffer import') +endfunc + +func Test_python3complete_allow_import_on_runs_imports() + " Symmetric positive control: with the flag set to non-zero, the harvested + " import IS exec()'d and the module loads. Without this control the + " negative test above could pass for unrelated reasons (e.g. completion + " failing to parse the buffer at all). + call assert_true(s:RunImportCompletion(1), + \ 'g:pythoncomplete_allow_import=1 did not run the buffer import') +endfunc + +" vim: shiftwidth=2 sts=2 expandtab diff --git a/src/version.c b/src/version.c index b8863bad28..81bd8d09f4 100644 --- a/src/version.c +++ b/src/version.c @@ -729,6 +729,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 597, /**/ 596, /**/