From 581e4154190d096c3640b468f91d60b1f362f45f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 7 Mar 2022 19:59:04 +0800 Subject: [PATCH 01/19] Allow render HTML with css/js external links --- modules/markup/csv/csv.go | 5 ++ modules/markup/external/external.go | 5 ++ modules/markup/markdown/markdown.go | 5 ++ modules/markup/orgmode/orgmode.go | 5 ++ modules/markup/renderer.go | 17 ++++++ modules/setting/markup.go | 2 + routers/web/repo/render.go | 85 +++++++++++++++++++++++++++++ routers/web/repo/view.go | 5 +- routers/web/web.go | 7 +++ 9 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 routers/web/repo/render.go diff --git a/modules/markup/csv/csv.go b/modules/markup/csv/csv.go index 17c3fe6f4f25c..609cf5cf34e23 100644 --- a/modules/markup/csv/csv.go +++ b/modules/markup/csv/csv.go @@ -51,6 +51,11 @@ func (Renderer) SanitizerDisabled() bool { return false } +// DisplayInIFrame disabled sanitize if return true +func (Renderer) DisplayInIFrame() bool { + return false +} + func writeField(w io.Writer, element, class, field string) error { if _, err := io.WriteString(w, "<"); err != nil { return err diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 4fdd4315bc3e4..adfff874229ed 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -59,6 +59,11 @@ func (p *Renderer) SanitizerDisabled() bool { return p.DisableSanitizer } +// DisplayInIFrame disabled sanitize if return true +func (p *Renderer) DisplayInIFrame() bool { + return p.UseIFrame +} + func envMark(envName string) string { if runtime.GOOS == "windows" { return "%" + envName + "%" diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index 320c2f7f82782..61aeec3c3160f 100644 --- a/modules/markup/markdown/markdown.go +++ b/modules/markup/markdown/markdown.go @@ -226,6 +226,11 @@ func (Renderer) SanitizerDisabled() bool { return false } +// DisplayInIFrame disabled sanitize if return true +func (Renderer) DisplayInIFrame() bool { + return false +} + // Render implements markup.Renderer func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { return render(ctx, input, output) diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index 2f394b992b22b..f9d255d194f8d 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -52,6 +52,11 @@ func (Renderer) SanitizerDisabled() bool { return false } +// DisplayInIFrame disabled sanitize if return true +func (Renderer) DisplayInIFrame() bool { + return false +} + // Render renders orgmode rawbytes to HTML func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { htmlWriter := org.NewHTMLWriter() diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index cf8b9bace70ba..c815609baa28c 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -45,6 +45,7 @@ type RenderContext struct { GitRepo *git.Repository ShaExistCache map[string]bool cancelFn func() + UseIframe bool } // Cancel runs any cleanup functions that have been registered for this Ctx @@ -82,6 +83,7 @@ type Renderer interface { NeedPostProcess() bool SanitizerRules() []setting.MarkupSanitizerRule SanitizerDisabled() bool + DisplayInIFrame() bool Render(ctx *RenderContext, input io.Reader, output io.Writer) error } @@ -134,6 +136,18 @@ type nopCloser struct { func (nopCloser) Close() error { return nil } +func renderIFrame(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error { + _, err := io.WriteString(output, fmt.Sprintf(``, + setting.AppSubURL, + url.PathEscape(ctx.Metas["user"]), + url.PathEscape(ctx.Metas["repo"]), ctx.Metas["BranchNameSubURL"], url.PathEscape(ctx.RelativePath), )) diff --git a/routers/web/repo/render.go b/routers/web/repo/render.go index 22a1d4dc73739..5452f2848ba69 100644 --- a/routers/web/repo/render.go +++ b/routers/web/repo/render.go @@ -9,7 +9,6 @@ import ( "io" "net/http" "path" - "strings" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/context" @@ -64,22 +63,15 @@ func RenderFile(ctx *context.Context) { treeLink += "/" + util.PathEscapeSegments(ctx.Repo.TreePath) } - var result bytes.Buffer err = markup.Render(&markup.RenderContext{ Ctx: ctx, RelativePath: ctx.Repo.TreePath, URLPrefix: path.Dir(treeLink), Metas: ctx.Repo.Repository.ComposeDocumentMetas(), GitRepo: ctx.Repo.GitRepo, - }, rd, &result) + }, rd, ctx.Resp) if err != nil { ctx.ServerError("Render", err) return } - - _, err = charset.EscapeControlReader(strings.NewReader(result.String()), ctx.Resp) - if err != nil { - ctx.ServerError("EscapeControlReader", err) - return - } } From feb290aea984408b065c4504f55c9e215ee70e2b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 12 Jun 2022 20:10:08 +0800 Subject: [PATCH 10/19] some improvement --- modules/markup/console/console.go | 17 +++-------------- modules/markup/csv/csv.go | 17 +++-------------- modules/markup/external/external.go | 4 ++-- modules/markup/markdown/markdown.go | 14 +++----------- modules/markup/orgmode/orgmode.go | 14 +++----------- modules/markup/renderer.go | 11 +++++++++++ 6 files changed, 25 insertions(+), 52 deletions(-) diff --git a/modules/markup/console/console.go b/modules/markup/console/console.go index d1242b30d0298..06298c72c4160 100644 --- a/modules/markup/console/console.go +++ b/modules/markup/console/console.go @@ -26,16 +26,15 @@ func init() { } // Renderer implements markup.Renderer -type Renderer struct{} +type Renderer struct { + markup.BaseRenderer +} // Name implements markup.Renderer func (Renderer) Name() string { return MarkupName } -// NeedPostProcess implements markup.Renderer -func (Renderer) NeedPostProcess() bool { return false } - // Extensions implements markup.Renderer func (Renderer) Extensions() []string { return []string{".sh-session"} @@ -48,16 +47,6 @@ func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { } } -// SanitizerDisabled disabled sanitize if return true -func (Renderer) SanitizerDisabled() bool { - return false -} - -// DisplayInIFrame disabled sanitize if return true -func (Renderer) DisplayInIFrame() bool { - return false -} - // CanRender implements markup.RendererContentDetector func (Renderer) CanRender(filename string, input io.Reader) bool { buf, err := io.ReadAll(input) diff --git a/modules/markup/csv/csv.go b/modules/markup/csv/csv.go index 609cf5cf34e23..269816278d51d 100644 --- a/modules/markup/csv/csv.go +++ b/modules/markup/csv/csv.go @@ -22,16 +22,15 @@ func init() { } // Renderer implements markup.Renderer for csv files -type Renderer struct{} +type Renderer struct { + markup.BaseRenderer +} // Name implements markup.Renderer func (Renderer) Name() string { return "csv" } -// NeedPostProcess implements markup.Renderer -func (Renderer) NeedPostProcess() bool { return false } - // Extensions implements markup.Renderer func (Renderer) Extensions() []string { return []string{".csv", ".tsv"} @@ -46,16 +45,6 @@ func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { } } -// SanitizerDisabled disabled sanitize if return true -func (Renderer) SanitizerDisabled() bool { - return false -} - -// DisplayInIFrame disabled sanitize if return true -func (Renderer) DisplayInIFrame() bool { - return false -} - func writeField(w io.Writer, element, class, field string) error { if _, err := io.WriteString(w, "<"); err != nil { return err diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 0803d97728793..31246d3d6f682 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -56,10 +56,10 @@ func (p *Renderer) SanitizerRules() []setting.MarkupSanitizerRule { // SanitizerDisabled disabled sanitize if return true func (p *Renderer) SanitizerDisabled() bool { - return p.DisableSanitizer + return !p.UseIFrame && p.DisableSanitizer } -// DisplayInIFrame disabled sanitize if return true +// DisplayInIFrame represents whether render the content with an iframe func (p *Renderer) DisplayInIFrame() bool { return p.UseIFrame } diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index ab5f87d5e6b8e..f52c5bc97bd0d 100644 --- a/modules/markup/markdown/markdown.go +++ b/modules/markup/markdown/markdown.go @@ -203,7 +203,9 @@ func init() { } // Renderer implements markup.Renderer -type Renderer struct{} +type Renderer struct { + markup.BaseRenderer +} // Name implements markup.Renderer func (Renderer) Name() string { @@ -223,16 +225,6 @@ func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{} } -// SanitizerDisabled disabled sanitize if return true -func (Renderer) SanitizerDisabled() bool { - return false -} - -// DisplayInIFrame disabled sanitize if return true -func (Renderer) DisplayInIFrame() bool { - return false -} - // Render implements markup.Renderer func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { return render(ctx, input, output) diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index f9d255d194f8d..04b4b43196d04 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -27,7 +27,9 @@ func init() { } // Renderer implements markup.Renderer for orgmode -type Renderer struct{} +type Renderer struct { + markup.BaseRenderer +} // Name implements markup.Renderer func (Renderer) Name() string { @@ -47,16 +49,6 @@ func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{} } -// SanitizerDisabled disabled sanitize if return true -func (Renderer) SanitizerDisabled() bool { - return false -} - -// DisplayInIFrame disabled sanitize if return true -func (Renderer) DisplayInIFrame() bool { - return false -} - // Render renders orgmode rawbytes to HTML func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { htmlWriter := org.NewHTMLWriter() diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index 9c04ec5618cd5..07d7778b0e48d 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -97,6 +97,17 @@ type Renderer interface { Render(ctx *RenderContext, input io.Reader, output io.Writer) error } +type BaseRenderer struct{} + +// NeedPostProcess implements markup.Renderer +func (BaseRenderer) NeedPostProcess() bool { return false } + +// SanitizerDisabled disabled sanitize if return true +func (BaseRenderer) SanitizerDisabled() bool { return false } + +// DisplayInIFrame represents whether render the content with an iframe +func (BaseRenderer) DisplayInIFrame() bool { return false } + // RendererContentDetector detects if the content can be rendered // by specified renderer type RendererContentDetector interface { From 4520ea9e506cf64ea8b5913b21c4c841ea6e96f0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 12 Jun 2022 21:19:37 +0800 Subject: [PATCH 11/19] revert change in SanitizerDisabled of external renderer --- modules/markup/external/external.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 31246d3d6f682..8354c676ecc46 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -56,7 +56,7 @@ func (p *Renderer) SanitizerRules() []setting.MarkupSanitizerRule { // SanitizerDisabled disabled sanitize if return true func (p *Renderer) SanitizerDisabled() bool { - return !p.UseIFrame && p.DisableSanitizer + return p.DisableSanitizer } // DisplayInIFrame represents whether render the content with an iframe From a69ec31726b1097799f62d6bc9fd61cfb8e9deda Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 13 Jun 2022 15:21:34 +0800 Subject: [PATCH 12/19] Add sandbox for iframe and support allow-scripts and allow-same-origin --- modules/markup/renderer.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index 07d7778b0e48d..7f5e9dcaef595 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -179,7 +179,8 @@ func (nopCloser) Close() error { return nil } func renderIFrame(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error { _, err := io.WriteString(output, fmt.Sprintf(``, +onload="this.height=ifd.document.body.scrollHeight" width="100%%" scrolling="no" frameborder="0" style="overflow: hidden" +sandbox="allow-same-origin allow-scripts">`, setting.AppSubURL, url.PathEscape(ctx.Metas["user"]), url.PathEscape(ctx.Metas["repo"]), From 7d0e74eb1aaf0081113460a1d4d66533d764109f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 14 Jun 2022 11:41:36 +0800 Subject: [PATCH 13/19] refactor --- custom/conf/app.example.ini | 5 +- .../doc/advanced/config-cheat-sheet.en-us.md | 2 +- modules/markup/console/console.go | 4 +- modules/markup/csv/csv.go | 4 +- modules/markup/external/external.go | 3 + modules/markup/markdown/markdown.go | 8 +- modules/markup/orgmode/orgmode.go | 8 +- modules/markup/renderer.go | 75 +++++++++++-------- routers/web/repo/render.go | 11 +-- routers/web/repo/view.go | 1 - 10 files changed, 68 insertions(+), 53 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 5814c0d69d9ff..f9eea4fc7a578 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2181,8 +2181,11 @@ PATH = ;RENDER_COMMAND = "asciidoc --out-file=- -" ;; Don't pass the file on STDIN, pass the filename as argument instead. ;IS_INPUT_FILE = false -; Don't filter html tags and attributes if true +;; Don't filter html tags and attributes if true ;DISABLE_SANITIZER = false +;; Display the HTML with an embed iframe instead of rendering inside the page. When a renderer uses iframe +;; it suppresses DISABLE_SANITIZER option and there will be no sanitizer. +;USE_IFRAME=false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index c4c81d58ace80..cee27dc57172c 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -1027,7 +1027,7 @@ IS_INPUT_FILE = false - RENDER\_COMMAND: External command to render all matching extensions. - IS\_INPUT\_FILE: **false** Input is not a standard input but a file param followed `RENDER_COMMAND`. - DISABLE_SANITIZER: **false** Don't filter html tags and attributes if true. This is insecure. Don't change this to true except you know what that means. -- USE_IFRAME: **false** Display the HTML with an embed iframe but not directly renderer. +- USE_IFRAME: **false** Display the HTML with an embed iframe instead of rendering inside the page. When a renderer uses iframe, it suppresses DISABLE_SANITIZER option and there will be no sanitizer. Two special environment variables are passed to the render command: - `GITEA_PREFIX_SRC`, which contains the current URL prefix in the `src` path tree. To be used as prefix for links. diff --git a/modules/markup/console/console.go b/modules/markup/console/console.go index 06298c72c4160..597593eee153c 100644 --- a/modules/markup/console/console.go +++ b/modules/markup/console/console.go @@ -26,9 +26,7 @@ func init() { } // Renderer implements markup.Renderer -type Renderer struct { - markup.BaseRenderer -} +type Renderer struct{} // Name implements markup.Renderer func (Renderer) Name() string { diff --git a/modules/markup/csv/csv.go b/modules/markup/csv/csv.go index 269816278d51d..5095b854650d1 100644 --- a/modules/markup/csv/csv.go +++ b/modules/markup/csv/csv.go @@ -22,9 +22,7 @@ func init() { } // Renderer implements markup.Renderer for csv files -type Renderer struct { - markup.BaseRenderer -} +type Renderer struct{} // Name implements markup.Renderer func (Renderer) Name() string { diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 8354c676ecc46..7cd9ab4401515 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -34,6 +34,9 @@ type Renderer struct { *setting.MarkupRenderer } +var _ markup.PostProcessRenderer = (*Renderer)(nil) +var _ markup.ExternalRenderer = (*Renderer)(nil) + // Name returns the external tool name func (p *Renderer) Name() string { return p.MarkupName diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index f52c5bc97bd0d..37e11e606fce5 100644 --- a/modules/markup/markdown/markdown.go +++ b/modules/markup/markdown/markdown.go @@ -203,16 +203,16 @@ func init() { } // Renderer implements markup.Renderer -type Renderer struct { - markup.BaseRenderer -} +type Renderer struct{} + +var _ markup.PostProcessRenderer = (*Renderer)(nil) // Name implements markup.Renderer func (Renderer) Name() string { return MarkupName } -// NeedPostProcess implements markup.Renderer +// NeedPostProcess implements markup.PostProcessRenderer func (Renderer) NeedPostProcess() bool { return true } // Extensions implements markup.Renderer diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index 04b4b43196d04..8c9f3b3da736b 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -27,16 +27,16 @@ func init() { } // Renderer implements markup.Renderer for orgmode -type Renderer struct { - markup.BaseRenderer -} +type Renderer struct{} + +var _ markup.PostProcessRenderer = (*Renderer)(nil) // Name implements markup.Renderer func (Renderer) Name() string { return "orgmode" } -// NeedPostProcess implements markup.Renderer +// NeedPostProcess implements markup.PostProcessRenderer func (Renderer) NeedPostProcess() bool { return true } // Extensions implements markup.Renderer diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index 7f5e9dcaef595..21f974b71b714 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -44,18 +44,18 @@ type Header struct { // RenderContext represents a render context type RenderContext struct { - Ctx context.Context - RelativePath string // relative path from tree root of the branch - Type string - IsWiki bool - URLPrefix string - Metas map[string]string - DefaultLink string - GitRepo *git.Repository - ShaExistCache map[string]bool - cancelFn func() - TableOfContents []Header - AllowIFrame bool + Ctx context.Context + RelativePath string // relative path from tree root of the branch + Type string + IsWiki bool + URLPrefix string + Metas map[string]string + DefaultLink string + GitRepo *git.Repository + ShaExistCache map[string]bool + cancelFn func() + TableOfContents []Header + InStandalonePage bool // used by external render. the router "/org/repo/render/..." will output the rendered content in a standalone page } // Cancel runs any cleanup functions that have been registered for this Ctx @@ -90,23 +90,23 @@ func (ctx *RenderContext) AddCancel(fn func()) { type Renderer interface { Name() string // markup format name Extensions() []string - NeedPostProcess() bool SanitizerRules() []setting.MarkupSanitizerRule - SanitizerDisabled() bool - DisplayInIFrame() bool Render(ctx *RenderContext, input io.Reader, output io.Writer) error } -type BaseRenderer struct{} - -// NeedPostProcess implements markup.Renderer -func (BaseRenderer) NeedPostProcess() bool { return false } +// PostProcessRenderer defines an interface for renderers who need post process +type PostProcessRenderer interface { + NeedPostProcess() bool +} -// SanitizerDisabled disabled sanitize if return true -func (BaseRenderer) SanitizerDisabled() bool { return false } +// PostProcessRenderer defines an interface for external renderers +type ExternalRenderer interface { + // SanitizerDisabled disabled sanitize if return true + SanitizerDisabled() bool -// DisplayInIFrame represents whether render the content with an iframe -func (BaseRenderer) DisplayInIFrame() bool { return false } + // DisplayInIFrame represents whether render the content with an iframe + DisplayInIFrame() bool +} // RendererContentDetector detects if the content can be rendered // by specified renderer @@ -177,10 +177,14 @@ type nopCloser struct { func (nopCloser) Close() error { return nil } -func renderIFrame(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error { - _, err := io.WriteString(output, fmt.Sprintf(``, +func renderIFrame(ctx *RenderContext, output io.Writer) error { + _, err := io.WriteString(output, fmt.Sprintf(` +`, setting.AppSubURL, url.PathEscape(ctx.Metas["user"]), url.PathEscape(ctx.Metas["repo"]), @@ -202,7 +206,12 @@ func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Wr var pr2 io.ReadCloser var pw2 io.WriteCloser - if !renderer.SanitizerDisabled() { + var sanitizerDisabled bool + if r, ok := renderer.(ExternalRenderer); ok { + sanitizerDisabled = r.SanitizerDisabled() + } + + if !sanitizerDisabled { pr2, pw2 = io.Pipe() defer func() { _ = pr2.Close() @@ -221,7 +230,7 @@ func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Wr wg.Add(1) go func() { - if renderer.NeedPostProcess() { + if r, ok := renderer.(PostProcessRenderer); ok && r.NeedPostProcess() { err = PostProcess(ctx, pr, pw2) } else { _, err = io.Copy(pw2, pr) @@ -268,8 +277,12 @@ func (err ErrUnsupportedRenderExtension) Error() string { func renderFile(ctx *RenderContext, input io.Reader, output io.Writer) error { extension := strings.ToLower(filepath.Ext(ctx.RelativePath)) if renderer, ok := extRenderers[extension]; ok { - if renderer.DisplayInIFrame() && ctx.AllowIFrame { - return renderIFrame(ctx, renderer, input, output) + if r, ok := renderer.(ExternalRenderer); ok && r.DisplayInIFrame() { + if !ctx.InStandalonePage { + // for an external render, it could only output its content in a standalone page + // otherwise, a `, setting.AppSubURL, url.PathEscape(ctx.Metas["user"]), From 806e64736e8fc7fe13126d3d528cd93f77ec3d5d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 14 Jun 2022 21:34:46 +0800 Subject: [PATCH 15/19] fix lint --- modules/markup/external/external.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 7cd9ab4401515..7928e5802f492 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -34,8 +34,10 @@ type Renderer struct { *setting.MarkupRenderer } -var _ markup.PostProcessRenderer = (*Renderer)(nil) -var _ markup.ExternalRenderer = (*Renderer)(nil) +var ( + _ markup.PostProcessRenderer = (*Renderer)(nil) + _ markup.ExternalRenderer = (*Renderer)(nil) +) // Name returns the external tool name func (p *Renderer) Name() string { From 5a2416e263d2c5927ea4ed530317bd374fe661d3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 14 Jun 2022 21:36:25 +0800 Subject: [PATCH 16/19] fine tune --- modules/markup/renderer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index 980512afb0768..71d7d65809762 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -184,8 +184,8 @@ func renderIFrame(ctx *RenderContext, output io.Writer) error { `, setting.AppSubURL, url.PathEscape(ctx.Metas["user"]), From d9679cef89d3c7141d6cbc793d72402208f44f4b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 14 Jun 2022 23:19:10 +0800 Subject: [PATCH 17/19] use single option RENDER_CONTENT_MODE, use sandbox=allow-scripts --- custom/conf/app.example.ini | 10 ++--- .../doc/advanced/config-cheat-sheet.en-us.md | 8 ++-- .../doc/advanced/config-cheat-sheet.zh-cn.md | 8 ++-- modules/markup/external/external.go | 4 +- modules/markup/renderer.go | 4 +- modules/setting/markup.go | 39 ++++++++++++++----- 6 files changed, 49 insertions(+), 24 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index f9eea4fc7a578..9ce5e38ecd47d 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2181,11 +2181,11 @@ PATH = ;RENDER_COMMAND = "asciidoc --out-file=- -" ;; Don't pass the file on STDIN, pass the filename as argument instead. ;IS_INPUT_FILE = false -;; Don't filter html tags and attributes if true -;DISABLE_SANITIZER = false -;; Display the HTML with an embed iframe instead of rendering inside the page. When a renderer uses iframe -;; it suppresses DISABLE_SANITIZER option and there will be no sanitizer. -;USE_IFRAME=false +;; How the content will be rendered. +;; * sanitized: Sanitize the content and render it inside current page, only a few HTML tags and attributes are allowed. +;; * 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 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index cee27dc57172c..515575077d7f2 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -1026,14 +1026,16 @@ IS_INPUT_FILE = false command. Multiple extensions needs a comma as splitter. - RENDER\_COMMAND: External command to render all matching extensions. - IS\_INPUT\_FILE: **false** Input is not a standard input but a file param followed `RENDER_COMMAND`. -- DISABLE_SANITIZER: **false** Don't filter html tags and attributes if true. This is insecure. Don't change this to true except you know what that means. -- USE_IFRAME: **false** Display the HTML with an embed iframe instead of rendering inside the page. When a renderer uses iframe, it suppresses DISABLE_SANITIZER option and there will be no sanitizer. +- RENDER_CONTENT_MODE: **sanitized** How the content will be rendered. + - sanitized: Sanitize the content and render it inside current page, only a few HTML tags and attributes are allowed. + - 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. Two special environment variables are passed to the render command: - `GITEA_PREFIX_SRC`, which contains the current URL prefix in the `src` path tree. To be used as prefix for links. - `GITEA_PREFIX_RAW`, which contains the current URL prefix in the `raw` path tree. To be used as prefix for image paths. -If `DISABLE_SANITIZER` is false, Gitea supports customizing the sanitization policy for rendered HTML. The example below will support KaTeX output from pandoc. +If `RENDER_CONTENT_MODE` is `sanitized`, Gitea supports customizing the sanitization policy for rendered HTML. The example below will support KaTeX output from pandoc. ```ini [markup.sanitizer.TeX] diff --git a/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md b/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md index 56db54bc792ca..ef1504bc94fcb 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md +++ b/docs/content/doc/advanced/config-cheat-sheet.zh-cn.md @@ -318,15 +318,17 @@ IS_INPUT_FILE = false - FILE_EXTENSIONS: 关联的文档的扩展名,多个扩展名用都好分隔。 - RENDER_COMMAND: 工具的命令行命令及参数。 - IS_INPUT_FILE: 输入方式是最后一个参数为文件路径还是从标准输入读取。 -- DISABLE_SANITIZER: **false** 如果为 true 则不过滤 HTML 标签和属性。本功能可能引起 XSS 安全问题。除非你知道这意味着什么,否则不要设置为 true。 -- USE_IFRAME: **false** 采用iframe来渲染生成的HTML,还是直接在本页面渲染。 +- RENDER_CONTENT_MODE: **sanitized** 内容如何被渲染。 + - sanitized: 对内容进行净化并渲染到当前页面中,仅有一部分 HTML 标签和属性是被允许的。 + - no-sanitizer: 禁用净化器,把内容渲染到当前页面中。此模式是**不安全**的,如果内容中含有恶意代码,可能会导致 XSS 攻击。 + - iframe: 把内容渲染在一个独立的页面中并使用 iframe 嵌入到当前页面中。使用的 iframe 工作在沙箱模式并禁用了同源请求,JS 代码被安全的从父页面中隔离出去。 以下两个环境变量将会被传递给渲染命令: - `GITEA_PREFIX_SRC`:包含当前的`src`路径的URL前缀,可以被用于链接的前缀。 - `GITEA_PREFIX_RAW`:包含当前的`raw`路径的URL前缀,可以被用于图片的前缀。 -如果 `DISABLE_SANITIZER` 为 false,则 Gitea 支持自定义渲染 HTML 的净化策略。以下例子将用 pandoc 支持 KaTeX 输出。 +如果 `RENDER_CONTENT_MODE` 为 `sanitized`,则 Gitea 支持自定义渲染 HTML 的净化策略。以下例子将用 pandoc 支持 KaTeX 输出。 ```ini [markup.sanitizer.TeX] diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 7928e5802f492..23dd45ba0a1f2 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -61,12 +61,12 @@ func (p *Renderer) SanitizerRules() []setting.MarkupSanitizerRule { // SanitizerDisabled disabled sanitize if return true func (p *Renderer) SanitizerDisabled() bool { - return p.DisableSanitizer + 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.UseIFrame + return p.RenderContentMode == setting.RenderContentModeIframe } func envMark(envName string) string { diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index 71d7d65809762..e88fa311875d5 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -179,13 +179,15 @@ func (nopCloser) Close() error { return nil } func renderIFrame(ctx *RenderContext, output io.Writer) error { // set height="0" ahead, otherwise the scrollHeight would be max(150, realHeight) + // at the moment, only "allow-scripts" is allowed for sandbox mode. + // "allow-same-origin" should never be used, it leads to XSS attack, and it makes the JS in iframe can access parent window's config and CSRF token // TODO: when using dark theme, if the rendered content doesn't have proper style, the default text color is black, which is not easy to read _, err := io.WriteString(output, fmt.Sprintf(` `, setting.AppSubURL, url.PathEscape(ctx.Metas["user"]), diff --git a/modules/setting/markup.go b/modules/setting/markup.go index a9b0b2c489f05..fd41bdd7cc3a0 100644 --- a/modules/setting/markup.go +++ b/modules/setting/markup.go @@ -20,6 +20,12 @@ var ( MermaidMaxSourceCharacters int ) +const ( + RenderContentModeSanitized = "sanitized" + RenderContentModeNoSanitizer = "no-sanitizer" + RenderContentModeIframe = "iframe" +) + // MarkupRenderer defines the external parser configured in ini type MarkupRenderer struct { Enabled bool @@ -29,8 +35,7 @@ type MarkupRenderer struct { IsInputFile bool NeedPostProcess bool MarkupSanitizerRules []MarkupSanitizerRule - DisableSanitizer bool - UseIFrame bool + RenderContentMode string } // MarkupSanitizerRule defines the policy for whitelisting attributes on @@ -145,14 +150,28 @@ func newMarkupRenderer(name string, sec *ini.Section) { return } + if sec.HasKey("DISABLE_SANITIZER") { + log.Error("Deprecated setting `[markup.*]` `DISABLE_SANITIZER` present. This fallback will be removed in v1.18.0") + } + + renderContentMode := sec.Key("RENDER_CONTENT_MODE").MustString(RenderContentModeSanitized) + if !sec.HasKey("RENDER_CONTENT_MODE") && sec.Key("DISABLE_SANITIZER").MustBool(false) { + renderContentMode = RenderContentModeNoSanitizer // if only the legacy DISABLE_SANITIZER exists, use it + } + if renderContentMode != RenderContentModeSanitized && + renderContentMode != RenderContentModeNoSanitizer && + renderContentMode != RenderContentModeIframe { + log.Error("invalid RENDER_CONTENT_MODE: %q, default to %q", renderContentMode, RenderContentModeSanitized) + renderContentMode = RenderContentModeSanitized + } + ExternalMarkupRenderers = append(ExternalMarkupRenderers, &MarkupRenderer{ - Enabled: sec.Key("ENABLED").MustBool(false), - MarkupName: name, - FileExtensions: exts, - Command: command, - IsInputFile: sec.Key("IS_INPUT_FILE").MustBool(false), - NeedPostProcess: sec.Key("NEED_POSTPROCESS").MustBool(true), - DisableSanitizer: sec.Key("DISABLE_SANITIZER").MustBool(false), - UseIFrame: sec.Key("USE_IFRAME").MustBool(false), + Enabled: sec.Key("ENABLED").MustBool(false), + MarkupName: name, + FileExtensions: exts, + Command: command, + IsInputFile: sec.Key("IS_INPUT_FILE").MustBool(false), + NeedPostProcess: sec.Key("NEED_POSTPROCESS").MustBool(true), + RenderContentMode: renderContentMode, }) } From a1cac2a4de36c160260e9c9baeaecbf7e3f24c30 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 14 Jun 2022 23:49:25 +0800 Subject: [PATCH 18/19] fine tune CSP --- routers/web/repo/render.go | 1 + routers/web/repo/view.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/render.go b/routers/web/repo/render.go index 5fce28f39a220..28a6d2f4293cc 100644 --- a/routers/web/repo/render.go +++ b/routers/web/repo/render.go @@ -63,6 +63,7 @@ func RenderFile(ctx *context.Context) { treeLink += "/" + util.PathEscapeSegments(ctx.Repo.TreePath) } + ctx.Resp.Header().Add("Content-Security-Policy", "frame-src 'self'; sandbox allow-scripts") err = markup.Render(&markup.RenderContext{ Ctx: ctx, RelativePath: ctx.Repo.TreePath, diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 6f997a0273999..fe60cf44c7c7d 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -543,7 +543,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st return } // to prevent iframe load third-party url - ctx.Resp.Header().Add("Content-Security-Policy", "frame-src "+setting.AppURL) + ctx.Resp.Header().Add("Content-Security-Policy", "frame-src 'self'") ctx.Data["EscapeStatus"], ctx.Data["FileContent"] = charset.EscapeControlString(result.String()) } else if readmeExist && !shouldRenderSource { buf := &bytes.Buffer{} From cc18ebf059cf588e2b8c89f67cfe41953968cc2e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 15 Jun 2022 11:16:46 +0800 Subject: [PATCH 19/19] Apply suggestions from code review Co-authored-by: wxiaoguang --- custom/conf/app.example.ini | 2 +- docs/content/doc/advanced/config-cheat-sheet.en-us.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 9ce5e38ecd47d..03679edb8eb2e 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2182,7 +2182,7 @@ PATH = ;; Don't pass the file on STDIN, pass the filename as argument instead. ;IS_INPUT_FILE = false ;; How the content will be rendered. -;; * sanitized: Sanitize the content and render it inside current page, only a few HTML tags and attributes are allowed. +;; * 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 diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 515575077d7f2..ce4fe8c7959af 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -1027,7 +1027,7 @@ IS_INPUT_FILE = false - RENDER\_COMMAND: External command to render all matching extensions. - IS\_INPUT\_FILE: **false** Input is not a standard input but a file param followed `RENDER_COMMAND`. - RENDER_CONTENT_MODE: **sanitized** How the content will be rendered. - - sanitized: Sanitize the content and render it inside current page, only a few HTML tags and attributes are allowed. + - 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.