diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index b77da3ab79..5fee78af54 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2540,13 +2540,19 @@ LEVEL = Info ;; * sanitized: Sanitize the content and render it inside current page, default to only allow a few HTML tags and attributes. Customized sanitizer rules can be defined in [markup.sanitizer.*] . ;; * no-sanitizer: Disable the sanitizer and render the content inside current page. It's **insecure** and may lead to XSS attack if the content contains malicious code. ;; * iframe: Render the content in a separate standalone page and embed it into current page by iframe. The iframe is in sandbox mode with same-origin disabled, and the JS code are safely isolated from parent page. -;RENDER_CONTENT_MODE=sanitized -;; +;RENDER_CONTENT_MODE = sanitized +;; The sandbox applied to the iframe and Content-Security-Policy header when RENDER_CONTENT_MODE is `iframe`. +;; It defaults to a safe set of "allow-*" restrictions (space separated). +;; You can also set it by your requirements or use "disabled" to disable the sandbox completely. +;; When set it, make sure there is no security risk: +;; * PDF-only content: generally safe to use "disabled", and it needs to be "disabled" because PDF only renders with no sandbox. +;; * HTML content with JS: if the "RENDER_COMMAND" can guarantee there is no XSS, then it is safe, otherwise, you need to fine tune the "allow-*" restrictions. +;RENDER_CONTENT_SANDBOX = ;; Whether post-process the rendered HTML content, including: ;; resolve relative links and image sources, recognizing issue/commit references, escaping invisible characters, ;; mentioning users, rendering permlink code blocks, replacing emoji shorthands, etc. ;; By default, this is true when RENDER_CONTENT_MODE is `sanitized`, otherwise false. -;NEED_POST_PROCESS=false +;NEED_POST_PROCESS = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/modules/httplib/serve.go b/modules/httplib/serve.go index 7c1edf432d..b4c5e7fe1e 100644 --- a/modules/httplib/serve.go +++ b/modules/httplib/serve.go @@ -126,6 +126,7 @@ func setServeHeadersByFile(r *http.Request, w http.ResponseWriter, mineBuf []byt // no sandbox attribute for pdf as it breaks rendering in at least safari. this // should generally be safe as scripts inside PDF can not escape the PDF document // see https://bugs.chromium.org/p/chromium/issues/detail?id=413851 for more discussion + // HINT: PDF-RENDER-SANDBOX: PDF won't render in sandboxed context w.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'") } diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index be40a50aad..3cbe14b86a 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -58,14 +58,11 @@ func (p *Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return p.MarkupSanitizerRules } -// SanitizerDisabled disabled sanitize if return true -func (p *Renderer) SanitizerDisabled() bool { - return p.RenderContentMode == setting.RenderContentModeNoSanitizer || p.RenderContentMode == setting.RenderContentModeIframe -} - -// DisplayInIFrame represents whether render the content with an iframe -func (p *Renderer) DisplayInIFrame() bool { - return p.RenderContentMode == setting.RenderContentModeIframe +func (p *Renderer) GetExternalRendererOptions() (ret markup.ExternalRendererOptions) { + ret.SanitizerDisabled = p.RenderContentMode == setting.RenderContentModeNoSanitizer || p.RenderContentMode == setting.RenderContentModeIframe + ret.DisplayInIframe = p.RenderContentMode == setting.RenderContentModeIframe + ret.ContentSandbox = p.RenderContentSandbox + return ret } func envMark(envName string) string { diff --git a/modules/markup/internal/finalprocessor.go b/modules/markup/internal/finalprocessor.go index 14d46a161f..4442afa0c9 100644 --- a/modules/markup/internal/finalprocessor.go +++ b/modules/markup/internal/finalprocessor.go @@ -5,11 +5,13 @@ package internal import ( "bytes" + "html/template" "io" ) type finalProcessor struct { renderInternal *RenderInternal + extraHeadHTML template.HTML output io.Writer buf bytes.Buffer @@ -25,6 +27,32 @@ func (p *finalProcessor) Close() error { // because "postProcess" already does so. In the future we could optimize the code to process data on the fly. buf := p.buf.Bytes() buf = bytes.ReplaceAll(buf, []byte(` data-attr-class="`+p.renderInternal.secureIDPrefix), []byte(` class="`)) - _, err := p.output.Write(buf) + + tmp := bytes.TrimSpace(buf) + isLikelyHTML := len(tmp) != 0 && tmp[0] == '<' && tmp[len(tmp)-1] == '>' && bytes.Index(tmp, []byte(``)) > 0 + if !isLikelyHTML { + // not HTML, write back directly + _, err := p.output.Write(buf) + return err + } + + // add our extra head HTML into output + headBytes := []byte("
") + posHead := bytes.Index(buf, headBytes) + var part1, part2 []byte + if posHead >= 0 { + part1, part2 = buf[:posHead+len(headBytes)], buf[posHead+len(headBytes):] + } else { + part1, part2 = nil, buf + } + if len(part1) > 0 { + if _, err := p.output.Write(part1); err != nil { + return err + } + } + if _, err := io.WriteString(p.output, string(p.extraHeadHTML)); err != nil { + return err + } + _, err := p.output.Write(part2) return err } diff --git a/modules/markup/internal/internal_test.go b/modules/markup/internal/internal_test.go index 590bcbb67f..a216d75203 100644 --- a/modules/markup/internal/internal_test.go +++ b/modules/markup/internal/internal_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestRenderInternal(t *testing.T) { +func TestRenderInternalAttrs(t *testing.T) { cases := []struct { input, protected, recovered string }{ @@ -30,7 +30,7 @@ func TestRenderInternal(t *testing.T) { for _, c := range cases { var r RenderInternal out := &bytes.Buffer{} - in := r.init("sec", out) + in := r.init("sec", out, "") protected := r.ProtectSafeAttrs(template.HTML(c.input)) assert.EqualValues(t, c.protected, protected) _, _ = io.WriteString(in, string(protected)) @@ -41,7 +41,7 @@ func TestRenderInternal(t *testing.T) { var r1, r2 RenderInternal protected := r1.ProtectSafeAttrs(``) assert.EqualValues(t, ``, protected, "non-initialized RenderInternal should not protect any attributes") - _ = r1.init("sec", nil) + _ = r1.init("sec", nil, "") protected = r1.ProtectSafeAttrs(``) assert.EqualValues(t, ``, protected) assert.Equal(t, "data-attr-class", r1.SafeAttr("class")) @@ -54,8 +54,37 @@ func TestRenderInternal(t *testing.T) { assert.Empty(t, recovered) out2 := &bytes.Buffer{} - in2 := r2.init("sec-other", out2) + in2 := r2.init("sec-other", out2, "") _, _ = io.WriteString(in2, string(protected)) _ = in2.Close() assert.Equal(t, ``, out2.String(), "different secureID should not recover the value") } + +func TestRenderInternalExtraHead(t *testing.T) { + t.Run("HeadExists", func(t *testing.T) { + out := &bytes.Buffer{} + var r RenderInternal + in := r.init("sec", out, `