Permalink
Browse files

patch 8.0.1641: job in terminal can't communicate with Vim

Problem:    Job in terminal can't communicate with Vim.
Solution:   Add the terminal API.
  • Loading branch information...
brammool committed Mar 25, 2018
1 parent 6587384 commit 8fbaeb195d9298c3a2a80300b5f96f1adddd2f59
Showing with 317 additions and 25 deletions.
  1. +95 −16 runtime/doc/terminal.txt
  2. +1 −1 src/buffer.c
  3. +141 −6 src/terminal.c
  4. +2 −2 src/testdir/screendump.vim
  5. +76 −0 src/testdir/test_terminal.vim
  6. +2 −0 src/version.c
@@ -1,4 +1,4 @@
*terminal.txt* For Vim version 8.0. Last change: 2018 Mar 16
*terminal.txt* For Vim version 8.0. Last change: 2018 Mar 25
VIM REFERENCE MANUAL by Bram Moolenaar
@@ -23,12 +23,16 @@ If the result is "1" you have it.
Session |terminal-session|
Unix |terminal-unix|
MS-Windows |terminal-ms-windows|
2. Remote testing |terminal-testing|
3. Diffing screen dumps |terminal-diff|
2. Terminal communication |terminal-communication|
Vim to job: term_sendkeys() |terminal-to-job|
Job to Vim: JSON API |terminal-api|
Using the client-server feature |terminal-client-server|
3. Remote testing |terminal-testing|
4. Diffing screen dumps |terminal-diff|
Writing a screen dump test for Vim |terminal-dumptest|
Creating a screen dump |terminal-screendump|
Comparing screen dumps |terminal-diffscreendump|
4. Debugging |terminal-debug|
5. Debugging |terminal-debug|
Starting |termdebug-starting|
Example session |termdebug-example|
Stepping through code |termdebug-stepping|
@@ -355,15 +359,6 @@ Environment variables are used to pass information to the running job:
COLORS number of colors, 't_Co' (256*256*256 in the GUI)
VIM_SERVERNAME v:servername
The |client-server| feature can be used to communicate with the Vim instance
where the job was started. This only works when v:servername is not empty.
If needed you can set it with: >
call remote_startserver('vim-server')
In the job you can then do something like: >
vim --servername $VIM_SERVERNAME --remote +123 some_file.c
This will open the file "some_file.c" and put the cursor on line 123.
MS-Windows ~
*terminal-ms-windows*
@@ -389,7 +384,91 @@ Environment variables are used to pass information to the running job:
VIM_SERVERNAME v:servername
==============================================================================
2. Remote testing *terminal-testing*
2. Terminal communication *terminal-communication*
There are several ways to communicate with the job running in a terminal:
- Use |term_sendkeys()| to send text and escape sequences from Vim to the job.
- Use the JSON API to send encoded commands from the job to Vim.
- Use the |client-server| mechanism. This works on machines with an X server
and on MS-Windows.
Vim to job: term_sendkeys() ~
*terminal-to-job*
This allows for remote controlling the job running in the terminal. It is a
one-way mechanism. The job can update the display to signal back to Vim.
For example, if a shell is running in a terminal, you can do: >
call term_sendkeys(buf, "ls *.java\<CR>")
This requires for the job to be in the right state where it will do the right
thing when receiving the keys. For the above example, the shell must be
waiting for a command to be typed.
For a job that was written for the purpose, you can use the JSON API escape
sequence in the other direction. E.g.: >
call term_sendkeys(buf, "\<Esc>]51;["response"]\x07")
Job to Vim: JSON API ~
*terminal-api*
The job can send JSON to Vim, using a special escape sequence. The JSON
encodes a command that Vim understands. Example of such a message: >
<Esc>]51;["drop", "README.md"]<07>
The body is always a list, making it easy to find the end: ]<07>.
The <Esc>]51;msg<07> sequence is reserved by xterm for "Emacs shell", which is
similar to what we are doing here.
Currently supported commands:
call {funcname} {argument}
Call a user defined function with [argument]. The function is
called with the buffer number of the terminal and the decoded
argument. The user function must sanity check the argument.
The function can use |term_sendkeys()| to send back a reply.
Example in JSON: >
["call", "Impression", ["play", 14]]
< Calls a function defined like this: >
function Impression(bufnum, arglist)
if len(a:arglist) == 2
echo "impression " . a:arglist[0]
echo "count " . a:arglist[1]
endif
endfunc
<
drop {filename}
Let Vim open a file, like the `:drop` command. If {filename}
is already open in a window, switch to that window. Otherwise
open a new window to edit {filename}.
Example in JSON: >
["drop", "path/file.txt", {"ff": "dos"}]
A trick to have Vim send this escape sequence: >
exe "set t_ts=\<Esc>]51; t_fs=\x07"
let &titlestring = '["call","TryThis",["hello",123]]'
redraw
set t_ts& t_fs&
Rationale: Why not allow for any command or expression? Because that might
create a security problem.
Using the client-server feature ~
*terminal-client-server*
This only works when v:servername is not empty. If needed you can set it,
before opening the terminal, with: >
call remote_startserver('vim-server')
$VIM_SERVERNAME is set in the terminal to pass on the server name.
In the job you can then do something like: >
vim --servername $VIM_SERVERNAME --remote +123 some_file.c
This will open the file "some_file.c" and put the cursor on line 123.
==============================================================================
3. Remote testing *terminal-testing*
Most Vim tests execute a script inside Vim. For some tests this does not
work, running the test interferes with the code being tested. To avoid this
@@ -404,7 +483,7 @@ term_scrape() inspect terminal screen
==============================================================================
3. Diffing screen dumps *terminal-diff*
4. Diffing screen dumps *terminal-diff*
In some cases it can be bothersome to test that Vim displays the right
characters on the screen. E.g. with syntax highlighting. To make this
@@ -494,7 +573,7 @@ Alternatively, press "s" to swap the first and second dump. Do this several
times so that you can spot the difference in the context of the text.
==============================================================================
4. Debugging *terminal-debug*
5. Debugging *terminal-debug*
The Terminal debugging plugin can be used to debug a program with gdb and view
the source code in a Vim window. Since this is completely contained inside
@@ -948,7 +948,7 @@ clear_wininfo(buf_T *buf)
}
}
#if defined(FEAT_LISTCMDS) || defined(PROTO)
#if defined(FEAT_LISTCMDS) || defined(FEAT_TERMINAL) || defined(PROTO)
/*
* Go to another buffer. Handles the result of the ATTENTION dialog.
*/
@@ -38,12 +38,11 @@
* in tl_scrollback are no longer used.
*
* TODO:
* - Win32: In the GUI use a terminal emulator for :!cmd.
* - For the "drop" command accept another argument for options.
* - Add a way to set the 16 ANSI colors, to be used for 'termguicolors' and in
* the GUI.
* - Some way for the job running in the terminal to send a :drop command back
* to the Vim running the terminal. Should be usable by a simple shell or
* python script.
* - Win32: Make terminal used for :!cmd in the GUI work better. Allow for
* redirection.
* - implement term_setsize()
* - Copy text in the vterm to the Vim buffer once in a while, so that
* completion works.
@@ -3145,6 +3144,140 @@ init_default_colors(term_T *term)
}
}
/*
* Handles a "drop" command from the job in the terminal.
* "item" is the file name, "item->li_next" may have options.
*/
static void
handle_drop_command(listitem_T *item)
{
char_u *fname = get_tv_string(&item->li_tv);
int bufnr;
win_T *wp;
tabpage_T *tp;
exarg_T ea;
bufnr = buflist_add(fname, BLN_LISTED | BLN_NOOPT);
FOR_ALL_TAB_WINDOWS(tp, wp)
{
if (wp->w_buffer->b_fnum == bufnr)
{
/* buffer is in a window already, go there */
goto_tabpage_win(tp, wp);
return;
}
}
/* open in new window, like ":sbuffer N" */
vim_memset(&ea, 0, sizeof(ea));
ea.cmd = (char_u *)"sbuffer";
goto_buffer(&ea, DOBUF_FIRST, FORWARD, bufnr);
}
/*
* Handles a function call from the job running in a terminal.
* "item" is the function name, "item->li_next" has the arguments.
*/
static void
handle_call_command(term_T *term, channel_T *channel, listitem_T *item)
{
char_u *func;
typval_T argvars[2];
typval_T rettv;
int doesrange;
if (item->li_next == NULL)
{
ch_log(channel, "Missing function arguments for call");
return;
}
func = get_tv_string(&item->li_tv);
if (!ASCII_ISUPPER(*func))
{
ch_log(channel, "Invalid function name: %s", func);
return;
}
argvars[0].v_type = VAR_NUMBER;
argvars[0].vval.v_number = term->tl_buffer->b_fnum;
argvars[1] = item->li_next->li_tv;
if (call_func(func, STRLEN(func), &rettv,
2, argvars, /* argv_func */ NULL,
/* firstline */ 1, /* lastline */ 1,
&doesrange, /* evaluate */ TRUE,
/* partial */ NULL, /* selfdict */ NULL) == OK)
{
clear_tv(&rettv);
ch_log(channel, "Function %s called", func);
}
else
ch_log(channel, "Calling function %s failed", func);
}
/*
* Called by libvterm when it cannot recognize an OSC sequence.
* We recognize a terminal API command.
*/
static int
parse_osc(const char *command, size_t cmdlen, void *user)
{
term_T *term = (term_T *)user;
js_read_T reader;
typval_T tv;
channel_T *channel = term->tl_job == NULL ? NULL
: term->tl_job->jv_channel;
/* We recognize only OSC 5 1 ; {command} */
if (cmdlen < 3 || STRNCMP(command, "51;", 3) != 0)
return 0; /* not handled */
reader.js_buf = vim_strnsave((char_u *)command + 3, cmdlen - 3);
if (reader.js_buf == NULL)
return 1;
reader.js_fill = NULL;
reader.js_used = 0;
if (json_decode(&reader, &tv, 0) == OK
&& tv.v_type == VAR_LIST
&& tv.vval.v_list != NULL)
{
listitem_T *item = tv.vval.v_list->lv_first;
if (item == NULL)
ch_log(channel, "Missing command");
else
{
char_u *cmd = get_tv_string(&item->li_tv);
item = item->li_next;
if (item == NULL)
ch_log(channel, "Missing argument for %s", cmd);
else if (STRCMP(cmd, "drop") == 0)
handle_drop_command(item);
else if (STRCMP(cmd, "call") == 0)
handle_call_command(term, channel, item);
else
ch_log(channel, "Invalid command received: %s", cmd);
}
}
else
ch_log(channel, "Invalid JSON received");
vim_free(reader.js_buf);
clear_tv(&tv);
return 1;
}
static VTermParserCallbacks parser_fallbacks = {
NULL, /* text */
NULL, /* control */
NULL, /* escape */
NULL, /* csi */
parse_osc, /* osc */
NULL, /* dcs */
NULL /* resize */
};
/*
* Create a new vterm and initialize it.
*/
@@ -3153,6 +3286,7 @@ create_vterm(term_T *term, int rows, int cols)
{
VTerm *vterm;
VTermScreen *screen;
VTermState *state;
VTermValue value;
vterm = vterm_new(rows, cols);
@@ -3186,8 +3320,9 @@ create_vterm(term_T *term, int rows, int cols)
#else
value.boolean = 0;
#endif
vterm_state_set_termprop(vterm_obtain_state(vterm),
VTERM_PROP_CURSORBLINK, &value);
state = vterm_obtain_state(vterm);
vterm_state_set_termprop(state, VTERM_PROP_CURSORBLINK, &value);
vterm_state_set_unrecognised_fallbacks(state, &parser_fallbacks, term);
}
/*
@@ -38,8 +38,8 @@ func RunVimInTerminal(arguments, options)
endif
" Make a horizontal and vertical split, so that we can get exactly the right
" size terminal window. Works only when we currently have one window.
call assert_equal(1, winnr('$'))
" size terminal window. Works only when the current window is full width.
call assert_equal(&columns, winwidth(0))
split
vsplit
Oops, something went wrong.

5 comments on commit 8fbaeb1

@andymass

This comment has been minimized.

andymass replied Mar 25, 2018

Restricting to user-defined functions seems insufficient to guarantee security. There should be some registration with the terminal of which functions are allowed or at least more restrictions on the function's name.

Just curious, why not trigger an autocmd instead?

@brammool

This comment has been minimized.

Contributor

brammool replied Mar 25, 2018

@andymass

This comment has been minimized.

andymass replied Mar 25, 2018

How is an autocommand better than a function? I would probably make the autocommand call a function anyway.

The difference is that while many plugins may have already defined SomeFunction() with unknown side-effects, no plugins are currently listening on a hypothetical new autocmd TerminalAPICall. Instead of making a function, users/plugins would listen to a specified autocmd pattern. This is effectively a registration system which eliminates security issues because each autocmd handler is easy to identify and audit, whereas allowing function calls exposes an unknown and potentially large attack surface.

I'm not sure how much security is really needed.

Sure, maybe it's unlikely. However, as is, merely cat-ting a specially crafted file is now dangerous if an attacker finds any commonly used plugin which happens to expose a function that has the right signature (two arguments, the second being a list). I think it's at or greater than the level of concern of modelines, which have had many changes over time to address various potential attacks (e.g., the sandbox).

@mattn

This comment has been minimized.

mattn replied Mar 26, 2018

I also think this is danger. For example, job is started like tail -f /path/to/something-webserver.log and when the attacker send escape sequences to the server, vim will inject to code.

@brammool

This comment has been minimized.

Contributor

brammool replied Mar 26, 2018

Now added the requirement for the Tapi_ prefix, that should avoid calling any function that wasn't made for the terminal API.

Please sign in to comment.