fix: sanitize external render command line arguments (#39417)

modules/markup/external/external.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/url"
 	"os"
 	"strings"
 
@@ -90,6 +91,49 @@ func (p *Renderer) GetExternalRendererOptions() (ret markup.ExternalRendererOpti
 	return ret
 }
 
+func sanitizeCliArgUrl(s string) string {
+	u, err := url.Parse(s)
+	if err != nil {
+		return ""
+	}
+	isSafeShellChar := func(c byte) bool {
+		return '0' <= c && c <= '9' || 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' ||
+			c == '-' || c == '_' || c == '.' || c == '~' ||
+			c == '+' || c == '@' || c == '%' || c == ':' || c == '[' || c == ']' ||
+			c >= 128
+	}
+	for i := 0; i < len(u.Host); i++ {
+		if !isSafeShellChar(u.Host[i]) {
+			return ""
+		}
+	}
+	const hex = "0123456789ABCDEF"
+	pathFields := strings.Split(u.EscapedPath(), "/")
+	for idx, pathField := range pathFields {
+		pos := 0
+		for ; pos < len(pathField); pos++ {
+			if !isSafeShellChar(pathField[pos]) {
+				break
+			}
+		}
+		if pos == len(pathField) {
+			continue
+		}
+		b := make([]byte, pos, len(pathField)+10)
+		copy(b, pathField[:pos])
+		for ; pos < len(pathField); pos++ {
+			if isSafeShellChar(pathField[pos]) {
+				b = append(b, pathField[pos])
+			} else {
+				b = append(b, '%', hex[pathField[pos]>>4], hex[pathField[pos]&0xf])
+			}
+		}
+		pathFields[idx] = string(b)
+	}
+	u2 := &url.URL{Scheme: u.Scheme, Host: u.Host}
+	return u2.String() + strings.Join(pathFields, "/")
+}
+
 func (p *Renderer) prepareExternalCommand(vars map[string]string) (string, []string, error) {
 	fields, err := shellquote.Split(strings.TrimSpace(p.Command))
 	if err != nil {
@@ -110,15 +154,36 @@ func (p *Renderer) prepareExternalCommand(vars map[string]string) (string, []str
 	return fields[0], fields[1:], nil
 }
 
+func (p *Renderer) prepare(baseLinkSrc, baseLinkRaw string) (ret struct {
+	envs []string
+	prog string
+	args []string
+}, err error,
+) {
+	// Although the URLs are also passed via environment variables,
+	// we still pass them via command line arguments because a 3rd party render program may not read environment variables.
+	// In case some site admins would write wrong render commands like `sh -c "echo $VAR"`, we sanitize the URLs here to make up for their mistakes
+	cmdVars := map[string]string{
+		"GITEA_PREFIX_SRC": sanitizeCliArgUrl(baseLinkSrc),
+		"GITEA_PREFIX_RAW": sanitizeCliArgUrl(baseLinkRaw),
+	}
+	ret.prog, ret.args, err = p.prepareExternalCommand(cmdVars)
+	if err != nil {
+		return ret, err
+	}
+	ret.envs = append(
+		os.Environ(),
+		"GITEA_PREFIX_SRC="+baseLinkSrc,
+		"GITEA_PREFIX_RAW="+baseLinkRaw,
+	)
+	return ret, nil
+}
+
 // Render renders the data of the document to HTML via the external tool.
 func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
 	baseLinkSrc := ctx.RenderHelper.ResolveLink("", markup.LinkTypeDefault)
 	baseLinkRaw := ctx.RenderHelper.ResolveLink("", markup.LinkTypeRaw)
-	cmdVars := map[string]string{
-		"GITEA_PREFIX_SRC": baseLinkSrc,
-		"GITEA_PREFIX_RAW": baseLinkRaw,
-	}
-	cmdProg, cmdArgs, err := p.prepareExternalCommand(cmdVars)
+	prepared, err := p.prepare(baseLinkSrc, baseLinkRaw)
 	if err != nil {
 		return fmt.Errorf("invalid external render (%s) command %q: %w", p.Name(), p.Command, err)
 	}
@@ -140,18 +205,14 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.
 		if err != nil {
 			return fmt.Errorf("%s close temp file when rendering %s failed: %w", p.Name(), p.Command, err)
 		}
-		cmdArgs = append(cmdArgs, tmpFile.Name())
+		prepared.args = append(prepared.args, tmpFile.Name())
 	}
 
-	processCtx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("Render [%s] for %s", cmdProg, baseLinkSrc))
+	processCtx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("Render [%s] for %s", prepared.prog, baseLinkSrc))
 	defer finished()
 
-	cmd := process.CommandContext(processCtx, cmdProg, cmdArgs...)
-	cmd.Env = append(
-		os.Environ(),
-		"GITEA_PREFIX_SRC="+baseLinkSrc,
-		"GITEA_PREFIX_RAW="+baseLinkRaw,
-	)
+	cmd := process.CommandContext(processCtx, prepared.prog, prepared.args...)
+	cmd.Env = prepared.envs
 	if !p.IsInputFile {
 		cmd.Stdin = input
 	}
@@ -160,7 +221,7 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.
 	cmd.Stderr = &stderr
 
 	if err := cmd.Run(); err != nil {
-		return fmt.Errorf("%s render run command %s %v failed: %w\nStderr: %s", p.Name(), cmdProg, shellquote.Join(cmdArgs...), err, stderr.String())
+		return fmt.Errorf("%s render run command %s %v failed: %w\nStderr: %s", p.Name(), prepared.prog, shellquote.Join(prepared.args...), err, stderr.String())
 	}
 	return nil
 }

modules/markup/external/external_test.go

@@ -12,13 +12,31 @@ import (
 )
 
 func TestPrepareExternalCommand(t *testing.T) {
-	r := &Renderer{MarkupRenderer: &setting.MarkupRenderer{Command: ""}}
-	_, _, err := r.prepareExternalCommand(map[string]string{"KEY": "val"})
-	assert.ErrorContains(t, err, "no command")
+	t.Run("NoCommand", func(t *testing.T) {
+		r := &Renderer{MarkupRenderer: &setting.MarkupRenderer{Command: ""}}
+		_, _, err := r.prepareExternalCommand(map[string]string{"KEY": "val"})
+		assert.ErrorContains(t, err, "no command")
+	})
+	t.Run("ReplaceArgs", func(t *testing.T) {
+		r := &Renderer{MarkupRenderer: &setting.MarkupRenderer{Command: `"/foo bar/bin" --opt $KEY "$KEY" %KEY% other`}}
+		prog, args, err := r.prepareExternalCommand(map[string]string{"KEY": `a"b`})
+		assert.NoError(t, err)
+		assert.Equal(t, "/foo bar/bin", prog)
+		assert.Equal(t, []string{"--opt", `a"b`, `a"b`, `a"b`, "other"}, args)
+	})
+}
 
-	r = &Renderer{MarkupRenderer: &setting.MarkupRenderer{Command: `"/foo bar/bin" --opt $KEY "$KEY" %KEY% other`}}
-	prog, args, err := r.prepareExternalCommand(map[string]string{"KEY": `a"b`})
-	assert.NoError(t, err)
-	assert.Equal(t, "/foo bar/bin", prog)
-	assert.Equal(t, []string{"--opt", `a"b`, `a"b`, `a"b`, "other"}, args)
+func TestPrepare(t *testing.T) {
+	t.Run("Valid", func(t *testing.T) {
+		r := &Renderer{MarkupRenderer: &setting.MarkupRenderer{Command: "foo $GITEA_PREFIX_SRC $GITEA_PREFIX_RAW"}}
+		ret, err := r.prepare("https://example.com/src", "https://example.com/raw")
+		assert.NoError(t, err)
+		assert.Equal(t, []string{"https://example.com/src", "https://example.com/raw"}, ret.args)
+	})
+	t.Run("Sanitized", func(t *testing.T) {
+		r := &Renderer{MarkupRenderer: &setting.MarkupRenderer{Command: "foo $GITEA_PREFIX_SRC $GITEA_PREFIX_RAW"}}
+		ret, err := r.prepare("https://[::1%25foo]:3000/a%2fb/x$y", "https://$foo.com")
+		assert.NoError(t, err)
+		assert.Equal(t, []string{"https://[::1%25foo]:3000/a%2fb/x%24y", ""}, ret.args)
+	})
 }