ipynb: sanitize rendered HTML (#5996)

* ipynb: sanitize rendered HTML

Fixes #5170

* Remove hardcode URL

* Add tests

CHANGELOG.md

@@ -32,12 +32,14 @@ All notable changes to Gogs are documented in this file.
 - Configuration option `[auth] ENABLE_NOTIFY_MAIL` is deprecated and will end support in 0.13.0, please start using `[user] ENABLE_EMAIL_NOTIFICATION`.
 - Configuration option `[session] GC_INTERVAL_TIME` is deprecated and will end support in 0.13.0, please start using `[session] GC_INTERVAL`.
 - Configuration option `[session] SESSION_LIFE_TIME` is deprecated and will end support in 0.13.0, please start using `[session] MAX_LIFE_TIME`.
+- The name `-` is reserved and cannot be used for users or organizations.
 
 ### Fixed
 
 - [Security] Potential open redirection with i18n.
 - [Security] Potential ability to delete files outside a repository.
 - [Security] Potential ability to set primary email on others' behalf from their verified emails.
+- [Security] Potential XSS attack via `.ipynb`. [#5170](https://github.com/gogs/gogs/issues/5170)
 - [Security] Potential RCE on mirror repositories. [#5767](https://github.com/gogs/gogs/issues/5767)
 - [Security] Potential XSS attack with raw markdown API. [#5907](https://github.com/gogs/gogs/pull/5907)
 - Open/close milestone redirects to a 404 page. [#5677](https://github.com/gogs/gogs/issues/5677)

internal/app/api.go

@@ -0,0 +1,36 @@
+// Copyright 2020 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package app
+
+import (
+	"net/http"
+
+	"github.com/microcosm-cc/bluemonday"
+	"gopkg.in/macaron.v1"
+
+	"gogs.io/gogs/internal/context"
+)
+
+func ipynbSanitizer() *bluemonday.Policy {
+	p := bluemonday.UGCPolicy()
+	p.AllowAttrs("class", "data-prompt-number").OnElements("div")
+	p.AllowAttrs("class").OnElements("img")
+	p.AllowURLSchemes("data")
+	return p
+}
+
+func SanitizeIpynb() macaron.Handler {
+	p := ipynbSanitizer()
+
+	return func(c *context.Context) {
+		html, err := c.Req.Body().String()
+		if err != nil {
+			c.Error(err, "read body")
+			return
+		}
+
+		c.PlainText(http.StatusOK, p.Sanitize(html))
+	}
+}

internal/app/api_test.go

@@ -0,0 +1,95 @@
+// Copyright 2020 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package app
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_ipynbSanitizer(t *testing.T) {
+	p := ipynbSanitizer()
+
+	tests := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{
+			name: "allow 'class' and 'data-prompt-number' attributes",
+			input: `
+<div class="nb-notebook">
+    <div class="nb-worksheet">
+        <div class="nb-cell nb-markdown-cell">Hello world</div>
+        <div class="nb-cell nb-code-cell">
+            <div class="nb-input" data-prompt-number="4">
+            </div>
+        </div>
+    </div>
+</div>
+`,
+			want: `
+<div class="nb-notebook">
+    <div class="nb-worksheet">
+        <div class="nb-cell nb-markdown-cell">Hello world</div>
+        <div class="nb-cell nb-code-cell">
+            <div class="nb-input" data-prompt-number="4">
+            </div>
+        </div>
+    </div>
+</div>
+`,
+		},
+		{
+			name: "allow base64 encoded images",
+			input: `
+<div class="nb-output" data-prompt-number="4">
+    <img class="nb-image-output" src="data:image/png;base64,iVBORw0KGgoA"/>
+</div>
+`,
+			want: `
+<div class="nb-output" data-prompt-number="4">
+    <img class="nb-image-output" src="data:image/png;base64,iVBORw0KGgoA"/>
+</div>
+`,
+		},
+		{
+			name: "prevent XSS",
+			input: `
+<div class="nb-output" data-prompt-number="10">
+<div class="nb-html-output">
+<style>
+.output {
+align-items: center;
+background: #00ff00;
+}
+</style>
+<script>
+function test() {
+alert("test");
+}
+
+$(document).ready(test);
+</script>
+</div>
+</div>
+`,
+			want: `
+<div class="nb-output" data-prompt-number="10">
+<div class="nb-html-output">
+
+
+</div>
+</div>
+`,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			assert.Equal(t, test.want, p.Sanitize(test.input))
+		})
+	}
+}

internal/app/metrics.go

@@ -0,0 +1,29 @@
+// Copyright 2020 The Gogs Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package app
+
+import (
+	"net/http"
+
+	"gopkg.in/macaron.v1"
+
+	"gogs.io/gogs/internal/conf"
+	"gogs.io/gogs/internal/context"
+)
+
+func MetricsFilter() macaron.Handler {
+	return func(c *context.Context) {
+		if !conf.Prometheus.Enabled {
+			c.Status(http.StatusNotFound)
+			return
+		}
+
+		if !conf.Prometheus.EnableBasicAuth {
+			return
+		}
+
+		c.RequireBasicAuth(conf.Prometheus.BasicAuthUsername, conf.Prometheus.BasicAuthPassword)
+	}
+}

internal/cmd/web.go

@@ -29,6 +29,7 @@ import (
 	"gopkg.in/macaron.v1"
 	log "unknwon.dev/clog/v2"
 
+	"gogs.io/gogs/internal/app"
 	"gogs.io/gogs/internal/assets/public"
 	"gogs.io/gogs/internal/assets/templates"
 	"gogs.io/gogs/internal/conf"
@@ -665,16 +666,15 @@ func runWeb(c *cli.Context) error {
 		apiv1.RegisterRoutes(m)
 	}, ignSignIn)
 
+	// ***************************
+	// ----- Internal routes -----
+	// ***************************
 	m.Group("/-", func() {
-		if conf.Prometheus.Enabled {
-			m.Get("/metrics", func(c *context.Context) {
-				if !conf.Prometheus.EnableBasicAuth {
-					return
-				}
+		m.Get("/metrics", app.MetricsFilter(), promhttp.Handler()) // "/-/metrics"
 
-				c.RequireBasicAuth(conf.Prometheus.BasicAuthUsername, conf.Prometheus.BasicAuthPassword)
-			}, promhttp.Handler())
-		}
+		m.Group("/api", func() {
+			m.Post("/sanitize_ipynb", app.SanitizeIpynb()) // "/-/api/sanitize_ipynb"
+		})
 	})
 
 	// robots.txt

internal/db/user.go

@@ -498,7 +498,7 @@ func NewGhostUser() *User {
 }
 
 var (
-	reservedUsernames    = []string{"explore", "create", "assets", "css", "img", "js", "less", "plugins", "debug", "raw", "install", "api", "avatar", "user", "org", "help", "stars", "issues", "pulls", "commits", "repo", "template", "admin", "new", ".", ".."}
+	reservedUsernames    = []string{"-", "explore", "create", "assets", "css", "img", "js", "less", "plugins", "debug", "raw", "install", "api", "avatar", "user", "org", "help", "stars", "issues", "pulls", "commits", "repo", "template", "admin", "new", ".", ".."}
 	reservedUserPatterns = []string{"*.keys"}
 )
 

templates/base/head.tmpl

@@ -44,8 +44,8 @@
 
 	<!-- notebook.js for rendering ipython notebooks and marked.js for rendering markdown in notebooks -->
 	{{if .IsIPythonNotebook}}
-		<script src="{{AppSubURL}}/plugins/notebookjs-0.3.0/notebook.min.js"></script>
-		<script src="{{AppSubURL}}/plugins/marked-0.3.6/marked.min.js"></script>
+		<script src="{{AppSubURL}}/plugins/notebookjs-0.4.2/notebook.min.js"></script>
+		<script src="{{AppSubURL}}/plugins/marked-0.8.1/marked.min.js"></script>
 	{{end}}
 
 	{{if .RequireSimpleMDE}}

templates/repo/view_file.tmpl

@@ -41,25 +41,32 @@
 				{{if .FileContent}}{{.FileContent | Str2HTML}}{{end}}
 			{{else if .IsIPythonNotebook}}
 				<script>
-					var rendered = null;
 					$.getJSON("{{.RawFileLink}}", null, function(notebook_json) {
 						var notebook = nb.parse(notebook_json);
-						rendered = notebook.render();
-						$("#ipython-notebook").append(rendered);
-						$("#ipython-notebook code").each(function(i, block) {
-							$(block).addClass("py").addClass("python");
-							hljs.highlightBlock(block);
-						});
+						var rendered = notebook.render();
+						$.ajax({
+							type: "POST",
+							url: '{{AppSubURL}}/-/api/sanitize_ipynb',
+							data: rendered.outerHTML,
+							processData: false,
+							contentType: false,
+						}).done(function(data) {
+							$("#ipython-notebook").append(data);
+							$("#ipython-notebook code").each(function(i, block) {
+								$(block).addClass("py").addClass("python");
+								hljs.highlightBlock(block);
+							});
 
-						// Overwrite image method to append proper prefix to the source URL
-						var renderer = new marked.Renderer();
-						var context = '{{.RawFileLink}}';
-						context = context.substring(0, context.lastIndexOf("/"));
-						renderer.image = function (href, title, text) {
-							return `<img src="${context}/${href}"`
-						}
-						$("#ipython-notebook .nb-markdown-cell").each(function(i, markdown) {
-							$(markdown).html(marked($(markdown).html(), {renderer: renderer}));
+							// Overwrite image method to append proper prefix to the source URL
+							var renderer = new marked.Renderer();
+							var context = '{{.RawFileLink}}';
+							context = context.substring(0, context.lastIndexOf("/"));
+							renderer.image = function (href, title, text) {
+								return `<img src="${context}/${href}"`
+							};
+							$("#ipython-notebook .nb-markdown-cell").each(function(i, markdown) {
+								$(markdown).html(marked($(markdown).html(), {renderer: renderer}));
+							});
 						});
 					});
 				</script>