invokeMethods and echoCommand

[nadmaximus]

Expected behavior

When entering a command which contains text to invoke methods, the command is processed (in my case, sent to the server via socket.io). The cmd or method invocations should not fire at this point. In terms of user expectations, the command just typed is not being echoed back - it's just remaining behind.

Actual behavior

In fact the prompt and the command just entered is echoed back and cmd/method invocations fire immediately. This behavior interferes with entering any text containing cmd/method invocation strings. Furthermore, there is no opportunity to sanitize or escape the entered command before it is echoed back, which means the user's commandline is a vector for social-engineering attack (getting people to type [[ terminal::pause() ]] for example). This appears to be a "hole" in the ability to use these invocations safely.

My workaround is to turn off echoCommand and echo the prompt + sanitized command via code. I escape brackets thusly and insert zero-width non-join characters: command.replace(/\[/g, '[‌').replace(/\]/g, ']‌');

However this means I must replace all cases where echoCommand: true would apply such as with terminal.read.

Steps to reproduce

In an interpreter with invokeMethods: true; echoCommand: true; enter:
[[ terminal::clear() ]]

Browser and OS

firefox 60.8.0esr (64-bit), Debian stretch

[jcubic]

I don't think this is a bug, this is the same as Self XSS warning when you open devtools on Facebook page. If someone type what someone else is telling him to type this is not the problem of the terminal. Consider this, you have command "delete user" and someone is telling someone else to type this into terminal.

[jcubic]

The problem it seems that you're echo back what ever user is typing, because echoCommand is not invoking the command if you type it into terminal, only if you have this code:

function(command) {
   this.echo(command);
}

[nadmaximus]

The user responsibility side of it is acceptable. However, this does not address the fact that it is currently impossible to enter a command which contains an invocation string without it immediately being invoked as soon as the command is echoed back. What if you want to create a server-side macro which will be triggered by something server-side, and send text back for your terminal to echo, with substitutions etc. So if you want to create that macro via the terminal, with a command, it's currently impossible to do so without disabling and replacing the echoCommand functionality.

[nadmaximus]

My terminal code which is invoking upon echoCommand:

terminal = $('#terminal').terminal([
                // commands handled by the local terminal
                {
                    help: function() {
                        this.echo('This is not helpful, but it is a command handled by the local client.');
                    }
                },
                // all other commands passed to terminal server
                function(command) {
                    socket.emit('stdin', command);
                }
            ], {
                prompt: '',
                greetings: '[[gbio;black;green]Dramaterm v0.1 initialized...]',
                outputLimit: 100,
                invokeMethods: true,
                echoCommand: true,
            });

The interpreter function is not echoing, and the invocation is definitely happening when the terminal echos back the command (not when, ultimately, the server sends back a response to the command which was entered). I have no problem sanitizing it on the server side before it is ever going to reach a client terminal...this invocation is happening before the server has a chance.

[jcubic]

Sorry, you're right, echo command is invoking methods, it should not execute the command, I was testing with prism enabled that was escaping brackets. This should be easy to fix, because echo have option exec that can be set to false.

[jcubic]

Thanks for this report, you just make this project better. I've also found bug in echo(xxx, {exec: false}) it was not executing methods but it was removing the formatting.

TODO: create better testing for extended commands and it's options.

[nadmaximus]

Excellent. It is wonderful to work with a project that is alive! Thanks for making it so.

[jcubic]

Released in version 2.7.1 will publish to npm soon.