fix

Author: wxiaoguang

custom/conf/app.example.ini

@@ -2182,10 +2182,10 @@ ROUTER = console
 ;; * 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
-;; when RENDER_CONTENT_MODE is iframe, below two items will be avaible
 ;;
-;RENDER_CONTENT_IFRAME_SANDBOX=allow-scripts
-;RENDER_CONTENT_EXTERNAL_CSP=sandbox allow-scripts
+;; When RENDER_CONTENT_MODE is iframe, these two options are available
+;RENDER_CONTENT_IFRAME_SANDBOX="allow-scripts"
+;RENDER_CONTENT_EXTERNAL_CSP="iframe-src 'self'; sandbox allow-scripts"
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

docs/content/doc/advanced/config-cheat-sheet.en-us.md

@@ -1044,8 +1044,8 @@ IS_INPUT_FILE = false
   - 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_IFRAME_SANDBOX: **allow-scripts** When `RENDER_CONTENT_MODE` is `iframe`, this will be the allowed sandbox of iframe properties.
-- RENDER_CONTENT_EXTERNAL_CSP: **sandbox allow-scripts** When `RENDER_CONTENT_MODE` is `iframe`, this will be the allowed CSP of external renderer response.
+- RENDER_CONTENT_IFRAME_SANDBOX: **"allow-scripts"** When `RENDER_CONTENT_MODE` is `iframe`, this will be the allowed sandbox of iframe properties.
+- RENDER_CONTENT_EXTERNAL_CSP: **"iframe-src 'self'; sandbox allow-scripts"** When `RENDER_CONTENT_MODE` is `iframe`, this will be the allowed CSP of external renderer response.
 
 Two special environment variables are passed to the render command:
 

modules/markup/renderer.go

@@ -55,6 +55,11 @@ type RenderContext struct {
 	ShaExistCache   map[string]bool
 	cancelFn        func()
 	TableOfContents []Header
+
+	// InStandalonePage is used by external render. the router "/org/repo/render/..." will output the rendered content in a standalone page
+	// It is for maintenance and security purpose, to avoid rendering external JS into embedded page unexpectedly.
+	// The caller of the Render must set security headers correctly before setting it to true
+	InStandalonePage bool
 }
 
 // Cancel runs any cleanup functions that have been registered for this Ctx
@@ -98,18 +103,18 @@ type PostProcessRenderer interface {
 	NeedPostProcess() bool
 }
 
-// PostProcessRenderer defines an interface for external renderers
+// ExternalRenderer 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
 	DisplayInIFrame() bool
 
-	// IframeSandbox represents iframe sandbox allowed options
+	// IframeSandbox represents iframe sandbox attribute for the <iframe> tag
 	IframeSandbox() string
 
-	// ExternalCSP represents external render CSP
+	// ExternalCSP represents the Content-Security-Policy header for external render
 	ExternalCSP() string
 }
 
@@ -157,13 +162,14 @@ func DetectRendererType(filename string, input io.Reader) string {
 	return ""
 }
 
-// GetRenderer returned the renderer according type or relativepath
-func GetRenderer(tp, relativePath string) (Renderer, error) {
-	if tp != "" {
-		if renderer, ok := renderers[tp]; ok {
+// GetRenderer returned the renderer according type or relative path
+func GetRenderer(renderType, relativePath string) (Renderer, error) {
+	if renderType != "" {
+		if renderer, ok := renderers[renderType]; ok {
 			return renderer, nil
 		}
-		return nil, ErrUnsupportedRenderType{tp}
+		// FIXME: is it correct? if it returns here, then relativePath won't take effect
+		return nil, ErrUnsupportedRenderType{renderType}
 	}
 
 	if relativePath != "" {
@@ -173,7 +179,8 @@ func GetRenderer(tp, relativePath string) (Renderer, error) {
 		}
 		return nil, ErrUnsupportedRenderExtension{extension}
 	}
-	return nil, errors.New("Render options both filename and type missing")
+
+	return nil, errors.New("render options both filename and type missing")
 }
 
 // Render renders markup file to HTML with all specific handling stuff.
@@ -231,6 +238,13 @@ sandbox="%s"
 
 // RenderDirect renders markup file to HTML with all specific handling stuff.
 func RenderDirect(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error {
+	if r, ok := renderer.(ExternalRenderer); ok && r.DisplayInIFrame() {
+		// to prevent from rendering external JS into embedded page unexpectedly, which would lead to XSS attack
+		if !ctx.InStandalonePage {
+			return errors.New("external render with iframe can only render in standalone page")
+		}
+	}
+
 	var wg sync.WaitGroup
 	var err error
 	pr, pw := io.Pipe()

modules/setting/markup.go

@@ -36,7 +36,7 @@ type MarkupRenderer struct {
 	NeedPostProcess            bool
 	MarkupSanitizerRules       []MarkupSanitizerRule
 	RenderContentMode          string
-	RenderContentIframeSandbox string // allow-scripts
+	RenderContentIframeSandbox string
 	RenderContentExternalCSP   string
 }
 
@@ -177,6 +177,6 @@ func newMarkupRenderer(name string, sec *ini.Section) {
 		NeedPostProcess:            sec.Key("NEED_POSTPROCESS").MustBool(true),
 		RenderContentMode:          renderContentMode,
 		RenderContentIframeSandbox: sec.Key("RENDER_CONTENT_IFRAME_SANDBOX").MustString("allow-scripts"),
-		RenderContentExternalCSP:   sec.Key("RENDER_CONTENT_EXTERNAL_CSP").MustString("sandbox allow-scripts"),
+		RenderContentExternalCSP:   sec.Key("RENDER_CONTENT_EXTERNAL_CSP").MustString("iframe-src 'self'; sandbox allow-scripts"),
 	})
 }

routers/web/repo/render.go

@@ -5,21 +5,16 @@
 package repo
 
 import (
-	"bytes"
-	"fmt"
-	"io"
 	"net/http"
 	"path"
 
-	"code.gitea.io/gitea/modules/charset"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/markup"
-	"code.gitea.io/gitea/modules/typesniffer"
 	"code.gitea.io/gitea/modules/util"
 )
 
-// RenderFile renders a file by repos path
+// RenderFile uses an external render to render a file by repos path
 func RenderFile(ctx *context.Context) {
 	blob, err := ctx.Repo.Commit.GetBlobByPath(ctx.Repo.TreePath)
 	if err != nil {
@@ -38,53 +33,39 @@ func RenderFile(ctx *context.Context) {
 	}
 	defer dataRc.Close()
 
-	buf := make([]byte, 1024)
-	n, _ := util.ReadAtMost(dataRc, buf)
-	buf = buf[:n]
-
-	st := typesniffer.DetectContentType(buf)
-	isTextFile := st.IsText()
-
-	rd := charset.ToUTF8WithFallbackReader(io.MultiReader(bytes.NewReader(buf), dataRc))
-
-	if markupType := markup.Type(blob.Name()); markupType == "" {
-		if isTextFile {
-			_, err = io.Copy(ctx.Resp, rd)
-			if err != nil {
-				ctx.ServerError("Copy", err)
-			}
-			return
-		}
-		ctx.Error(http.StatusInternalServerError, "Unsupported file type render")
-		return
-	}
-
 	treeLink := ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL()
 	if ctx.Repo.TreePath != "" {
 		treeLink += "/" + util.PathEscapeSegments(ctx.Repo.TreePath)
 	}
 
-	var allowSameOriginStr string
-
 	renderer, err := markup.GetRenderer("", ctx.Repo.TreePath)
 	if err != nil {
 		ctx.ServerError("GetRenderer", err)
 		return
 	}
 
-	if r, ok := renderer.(markup.ExternalRenderer); ok {
-		allowSameOriginStr = r.ExternalCSP()
+	externalRender, ok := renderer.(markup.ExternalRenderer)
+	if !ok {
+		ctx.Error(http.StatusBadRequest, "External render only")
+		return
+	}
+
+	externalCSP := externalRender.ExternalCSP()
+	if externalCSP == "" {
+		ctx.Error(http.StatusBadRequest, "External render must have valid Content-Security-Header")
+		return
 	}
 
-	ctx.Resp.Header().Add("Content-Security-Policy", fmt.Sprintf("frame-src 'self'; %s", allowSameOriginStr))
+	ctx.Resp.Header().Add("Content-Security-Policy", externalCSP)
 
 	if err = markup.RenderDirect(&markup.RenderContext{
-		Ctx:          ctx,
-		RelativePath: ctx.Repo.TreePath,
-		URLPrefix:    path.Dir(treeLink),
-		Metas:        ctx.Repo.Repository.ComposeDocumentMetas(),
-		GitRepo:      ctx.Repo.GitRepo,
-	}, renderer, rd, ctx.Resp); err != nil {
+		Ctx:              ctx,
+		RelativePath:     ctx.Repo.TreePath,
+		URLPrefix:        path.Dir(treeLink),
+		Metas:            ctx.Repo.Repository.ComposeDocumentMetas(),
+		GitRepo:          ctx.Repo.GitRepo,
+		InStandalonePage: true,
+	}, renderer, dataRc, ctx.Resp); err != nil {
 		ctx.ServerError("RenderDirect", err)
 		return
 	}