diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 65efbdef300..4791bf1e3fc 100644 --- a/modules/markup/external/external.go +++ b/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 } diff --git a/modules/markup/external/external_test.go b/modules/markup/external/external_test.go index 19b1bf378cc..a74e87af1b9 100644 --- a/modules/markup/external/external_test.go +++ b/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) + }) }