From 6976b1dce45e89fa55b80f9ba5a92b23ffb7ad15 Mon Sep 17 00:00:00 2001 From: Frank LENORMAND Date: Wed, 16 Sep 2020 11:37:01 +0300 Subject: [PATCH 1/7] rc modeline: Prevent command execution This commit prevents specially crafted modelines from making the editor execute arbitrary Kakoune commands. By using a tabulation character as a separator, commands can be injected in the value of the options listed in the modeline. For example: # kak: tabstop=2;set-option buffer pwned yes Fixes #3735. --- rc/detection/modeline.kak | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/rc/detection/modeline.kak b/rc/detection/modeline.kak index 00d2fcdc..3cf79b20 100644 --- a/rc/detection/modeline.kak +++ b/rc/detection/modeline.kak @@ -12,6 +12,8 @@ declare-option -docstring "amount of lines that will be checked at the beginning define-command -hidden modeline-parse-impl %{ evaluate-commands %sh{ + kakquote() { printf "%s" "$*" | sed "s/'/''/g; 1s/^/'/; \$s/\$/'/"; } + # Translate a vim option into the corresponding kakoune one translate_opt_vim() { readonly key="$1" @@ -19,11 +21,11 @@ define-command -hidden modeline-parse-impl %{ tr="" case "${key}" in - so|scrolloff) tr="scrolloff ${value},${kak_opt_scrolloff##*,}";; - siso|sidescrolloff) tr="scrolloff ${kak_opt_scrolloff%%,*},${value}";; - ts|tabstop) tr="tabstop ${value}";; - sw|shiftwidth) tr="indentwidth ${value}";; - tw|textwidth) tr="autowrap_column ${value}";; + so|scrolloff) tr=$(kakquote scrolloff "${value},${kak_opt_scrolloff##*,}");; + siso|sidescrolloff) tr=$(kakquote scrolloff "${kak_opt_scrolloff%%,*},${value}");; + ts|tabstop) tr=$(kakquote tabstop "${value}");; + sw|shiftwidth) tr=$(kakquote indentwidth "${value}");; + tw|textwidth) tr=$(kakquote autowrap_column "${value}");; ff|fileformat) case "${value}" in unix) tr="eolformat lf";; @@ -31,10 +33,10 @@ define-command -hidden modeline-parse-impl %{ *) printf %s\\n "echo -debug 'Unsupported file format: ${value}'";; esac ;; - ft|filetype) tr="filetype ${value}";; + ft|filetype) tr=$(kakquote filetype "{value}");; bomb) tr="BOM utf8";; nobomb) tr="BOM none";; - spelllang|spl) tr="spell_lang ${value%%,*}";; + spelllang|spl) tr=$(kakquote spell_lang "{value%%,*}");; *) printf %s\\n "echo -debug 'Unsupported vim variable: ${key}'";; esac @@ -52,7 +54,7 @@ define-command -hidden modeline-parse-impl %{ return;; esac - printf %s\\n "set-option buffer ${key} ${value}" + printf 'set-option buffer %s' "$(kakquote "${key}" "${value}")" } case "${kak_selection}" in From d02cb43a8879836d070a9ef83513ed40ccd0641e Mon Sep 17 00:00:00 2001 From: Frank LENORMAND Date: Wed, 16 Sep 2020 11:39:43 +0300 Subject: [PATCH 2/7] test: Implement a regression test for #3735 --- test/regression/3735-modeline-arbitrary-code-execution/cmd | 3 +++ test/regression/3735-modeline-arbitrary-code-execution/in | 1 + test/regression/3735-modeline-arbitrary-code-execution/out | 1 + test/regression/3735-modeline-arbitrary-code-execution/rc | 3 +++ 4 files changed, 8 insertions(+) create mode 100644 test/regression/3735-modeline-arbitrary-code-execution/cmd create mode 100644 test/regression/3735-modeline-arbitrary-code-execution/in create mode 100644 test/regression/3735-modeline-arbitrary-code-execution/out create mode 100644 test/regression/3735-modeline-arbitrary-code-execution/rc diff --git a/test/regression/3735-modeline-arbitrary-code-execution/cmd b/test/regression/3735-modeline-arbitrary-code-execution/cmd new file mode 100644 index 00000000..fefcd4a9 --- /dev/null +++ b/test/regression/3735-modeline-arbitrary-code-execution/cmd @@ -0,0 +1,3 @@ +:modeline-parse +% +:exec c %opt{pwned} diff --git a/test/regression/3735-modeline-arbitrary-code-execution/in b/test/regression/3735-modeline-arbitrary-code-execution/in new file mode 100644 index 00000000..4393d125 --- /dev/null +++ b/test/regression/3735-modeline-arbitrary-code-execution/in @@ -0,0 +1 @@ +# kak: tabstop=2;set-option buffer pwned yes diff --git a/test/regression/3735-modeline-arbitrary-code-execution/out b/test/regression/3735-modeline-arbitrary-code-execution/out new file mode 100644 index 00000000..c508d536 --- /dev/null +++ b/test/regression/3735-modeline-arbitrary-code-execution/out @@ -0,0 +1 @@ +false diff --git a/test/regression/3735-modeline-arbitrary-code-execution/rc b/test/regression/3735-modeline-arbitrary-code-execution/rc new file mode 100644 index 00000000..69ee4408 --- /dev/null +++ b/test/regression/3735-modeline-arbitrary-code-execution/rc @@ -0,0 +1,3 @@ +source "%val{runtime}/rc/detection/modeline.kak" +declare-option -hidden bool pwned no +set-option global tabstop 42 From 7731fe4cb27d32ede8b4961b10c63645377fe4ad Mon Sep 17 00:00:00 2001 From: Frank LENORMAND Date: Wed, 16 Sep 2020 11:58:47 +0300 Subject: [PATCH 3/7] rc modeline: Print error messages correctly --- rc/detection/modeline.kak | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/rc/detection/modeline.kak b/rc/detection/modeline.kak index 3cf79b20..7c436061 100644 --- a/rc/detection/modeline.kak +++ b/rc/detection/modeline.kak @@ -30,14 +30,16 @@ define-command -hidden modeline-parse-impl %{ case "${value}" in unix) tr="eolformat lf";; dos) tr="eolformat crlf";; - *) printf %s\\n "echo -debug 'Unsupported file format: ${value}'";; + *) printf 'echo -debug %s' "$(kakquote "Unsupported file format: ${value}")" \ + | kak -p "${kak_session}";; esac ;; ft|filetype) tr=$(kakquote filetype "{value}");; bomb) tr="BOM utf8";; nobomb) tr="BOM none";; spelllang|spl) tr=$(kakquote spell_lang "{value%%,*}");; - *) printf %s\\n "echo -debug 'Unsupported vim variable: ${key}'";; + *) printf 'echo -debug %s' "$(kakquote "Unsupported vim variable: ${key}")" \ + | kak -p "${kak_session}";; esac [ -n "${tr}" ] && printf %s\\n "set-option buffer ${tr}" @@ -50,7 +52,8 @@ define-command -hidden modeline-parse-impl %{ case "${key}" in scrolloff|tabstop|indentwidth|autowrap_column|eolformat|filetype|BOM|spell_lang);; - *) printf %s\\n "echo -debug 'Unsupported kakoune variable: ${key}'" + *) printf 'echo -debug %s' "$(kakquote "Unsupported kakoune variable: ${key}")" \ + | kak -p "${kak_session}" return;; esac @@ -60,7 +63,8 @@ define-command -hidden modeline-parse-impl %{ case "${kak_selection}" in *vi:*|*vim:*) type_selection="vim";; *kak:*|*kakoune:*) type_selection="kakoune";; - *) echo "echo -debug Unsupported modeline format"; exit 1 ;; + *) printf 'echo -debug Unsupported modeline format' \ + | kak -p "${kak_session}"; exit 1 ;; esac # The following subshell will keep the actual options of the modeline, and strip: From 30ee97386d2bd8d0240803c8a0fb2f57285a9d4c Mon Sep 17 00:00:00 2001 From: Frank LENORMAND Date: Wed, 16 Sep 2020 12:05:30 +0300 Subject: [PATCH 4/7] rc modeline: Use more idiomatic shell --- rc/detection/modeline.kak | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/rc/detection/modeline.kak b/rc/detection/modeline.kak index 7c436061..516080db 100644 --- a/rc/detection/modeline.kak +++ b/rc/detection/modeline.kak @@ -42,7 +42,9 @@ define-command -hidden modeline-parse-impl %{ | kak -p "${kak_session}";; esac - [ -n "${tr}" ] && printf %s\\n "set-option buffer ${tr}" + if [ -n "${tr}" ]; then + printf 'set-option buffer %s\n' "${tr}" + fi } # Pass a few whitelisted options to kakoune directly @@ -82,7 +84,9 @@ define-command -hidden modeline-parse-impl %{ name_option="${option%%=*}" value_option="${option#*=}" - [ -z "${option}" ] && continue + if [ -z "${option}" ]; then + continue + fi case "${type_selection}" in vim) tr=$(translate_opt_vim "${name_option}" "${value_option}");; @@ -90,7 +94,9 @@ define-command -hidden modeline-parse-impl %{ *) tr="";; esac - [ -n "${tr}" ] && printf %s\\n "${tr}" + if [ -n "${tr}" ]; then + printf %s\\n "${tr}" + fi done } } From 2d78b0760d2d755321dbc3321cb2e47d5d08c90f Mon Sep 17 00:00:00 2001 From: Frank LENORMAND Date: Wed, 16 Sep 2020 12:10:05 +0300 Subject: [PATCH 5/7] rc modeline: Error out on unsupported formats This commit makes `:modeline-parse` grab all lines that look like modelines, and lets the parser deal with invalid formats. This allows actually printing an error on unsupported modelines formats, instead of ignoring them upfront. --- rc/detection/modeline.kak | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rc/detection/modeline.kak b/rc/detection/modeline.kak index 516080db..dc092a1c 100644 --- a/rc/detection/modeline.kak +++ b/rc/detection/modeline.kak @@ -65,7 +65,7 @@ define-command -hidden modeline-parse-impl %{ case "${kak_selection}" in *vi:*|*vim:*) type_selection="vim";; *kak:*|*kakoune:*) type_selection="kakoune";; - *) printf 'echo -debug Unsupported modeline format' \ + *) printf 'echo -debug %s' "$(kakquote "Unsupported modeline format: ${kak_selection}")" \ | kak -p "${kak_session}"; exit 1 ;; esac @@ -109,7 +109,7 @@ define-command -hidden modeline-parse-impl %{ define-command modeline-parse -docstring "Read and interpret vi-format modelines at the beginning/end of the buffer" %{ try %{ evaluate-commands -draft %{ execute-keys "s(?S)\A(.+\n){,%opt{modelines}}|(.+\n){,%opt{modelines}}\z" \ - s^\S*?\s+?(vim?|kak(oune)?):\s?[^\n]+ + s^\S*?\s+?\w+:\s?[^\n]+ evaluate-commands -draft -itersel modeline-parse-impl } } } From 2a51ebf10515f6511c8b4af2de03bb96551d75c2 Mon Sep 17 00:00:00 2001 From: Frank LENORMAND Date: Fri, 18 Sep 2020 14:47:21 +0300 Subject: [PATCH 6/7] rc modeline: Factorise the use of `kakquote` --- rc/detection/modeline.kak | 55 ++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/rc/detection/modeline.kak b/rc/detection/modeline.kak index dc092a1c..39f35966 100644 --- a/rc/detection/modeline.kak +++ b/rc/detection/modeline.kak @@ -16,35 +16,48 @@ define-command -hidden modeline-parse-impl %{ # Translate a vim option into the corresponding kakoune one translate_opt_vim() { - readonly key="$1" - readonly value="$2" + key="$1" + value="$2" tr="" case "${key}" in - so|scrolloff) tr=$(kakquote scrolloff "${value},${kak_opt_scrolloff##*,}");; - siso|sidescrolloff) tr=$(kakquote scrolloff "${kak_opt_scrolloff%%,*},${value}");; - ts|tabstop) tr=$(kakquote tabstop "${value}");; - sw|shiftwidth) tr=$(kakquote indentwidth "${value}");; - tw|textwidth) tr=$(kakquote autowrap_column "${value}");; + so|scrolloff) + key="scrolloff"; + value="${value},${kak_opt_scrolloff##*,}";; + siso|sidescrolloff) + key="scrolloff"; + value="${kak_opt_scrolloff%%,*},${value}";; + ts|tabstop) key="tabstop";; + sw|shiftwidth) key="indentwidth";; + tw|textwidth) key="autowrap_column";; ff|fileformat) + key="eolformat"; case "${value}" in - unix) tr="eolformat lf";; - dos) tr="eolformat crlf";; - *) printf 'echo -debug %s' "$(kakquote "Unsupported file format: ${value}")" \ - | kak -p "${kak_session}";; + unix) value="lf";; + dos) value="crlf";; + *) + printf 'echo -debug %s' "$(kakquote "Unsupported file format: ${value}")" \ + | kak -p "${kak_session}"; + return;; esac ;; - ft|filetype) tr=$(kakquote filetype "{value}");; - bomb) tr="BOM utf8";; - nobomb) tr="BOM none";; - spelllang|spl) tr=$(kakquote spell_lang "{value%%,*}");; - *) printf 'echo -debug %s' "$(kakquote "Unsupported vim variable: ${key}")" \ - | kak -p "${kak_session}";; + ft|filetype) key="filetype";; + bomb) + key="BOM"; + value="utf8";; + nobomb) + key="BOM"; + value="none";; + spelllang|spl) + key="spell_lang"; + value="${value%%,*}";; + *) + printf 'echo -debug %s' "$(kakquote "Unsupported vim variable: ${key}")" \ + | kak -p "${kak_session}"; + return;; esac - if [ -n "${tr}" ]; then - printf 'set-option buffer %s\n' "${tr}" - fi + printf 'set-option buffer %s %s\n' "${key}" "$(kakquote "${value}")" } # Pass a few whitelisted options to kakoune directly @@ -59,7 +72,7 @@ define-command -hidden modeline-parse-impl %{ return;; esac - printf 'set-option buffer %s' "$(kakquote "${key}" "${value}")" + printf 'set-option buffer %s %s\n' "${key}" "$(kakquote "${value}")" } case "${kak_selection}" in From 4025ac81673e148d3e6109a6052627af9b2660a3 Mon Sep 17 00:00:00 2001 From: Frank LENORMAND Date: Fri, 18 Sep 2020 14:50:08 +0300 Subject: [PATCH 7/7] rc modeline: Print the final command directly --- rc/detection/modeline.kak | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/rc/detection/modeline.kak b/rc/detection/modeline.kak index 39f35966..7bfd7852 100644 --- a/rc/detection/modeline.kak +++ b/rc/detection/modeline.kak @@ -102,14 +102,10 @@ define-command -hidden modeline-parse-impl %{ fi case "${type_selection}" in - vim) tr=$(translate_opt_vim "${name_option}" "${value_option}");; - kakoune) tr=$(translate_opt_kakoune "${name_option}" "${value_option}");; - *) tr="";; + vim) translate_opt_vim "${name_option}" "${value_option}";; + kakoune) translate_opt_kakoune "${name_option}" "${value_option}";; + *) exit 1;; esac - - if [ -n "${tr}" ]; then - printf %s\\n "${tr}" - fi done } }