Skip to content

Commit 69cb408

Browse files
unknwonmpsonntag
unknwon
authored and
mpsonntag
committed
ipynb: sanitize rendered HTML (#5996)
* ipynb: sanitize rendered HTML Fixes #5170 * Remove hardcode URL * Add tests
1 parent b92f7d8 commit 69cb408

File tree

15 files changed

+240
-67
lines changed

15 files changed

+240
-67
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,14 @@ All notable changes to Gogs are documented in this file.
3232
- Configuration option `[auth] ENABLE_NOTIFY_MAIL` is deprecated and will end support in 0.13.0, please start using `[user] ENABLE_EMAIL_NOTIFICATION`.
3333
- Configuration option `[session] GC_INTERVAL_TIME` is deprecated and will end support in 0.13.0, please start using `[session] GC_INTERVAL`.
3434
- Configuration option `[session] SESSION_LIFE_TIME` is deprecated and will end support in 0.13.0, please start using `[session] MAX_LIFE_TIME`.
35+
- The name `-` is reserved and cannot be used for users or organizations.
3536

3637
### Fixed
3738

3839
- [Security] Potential open redirection with i18n.
3940
- [Security] Potential ability to delete files outside a repository.
4041
- [Security] Potential ability to set primary email on others' behalf from their verified emails.
42+
- [Security] Potential XSS attack via `.ipynb`. [#5170](https://github.com/gogs/gogs/issues/5170)
4143
- [Security] Potential RCE on mirror repositories. [#5767](https://github.com/gogs/gogs/issues/5767)
4244
- [Security] Potential XSS attack with raw markdown API. [#5907](https://github.com/gogs/gogs/pull/5907)
4345
- Open/close milestone redirects to a 404 page. [#5677](https://github.com/gogs/gogs/issues/5677)

‎go.sum

+4
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ github.com/G-Node/libgin v0.0.0-20191216094436-47f8aadc0067/go.mod h1:2yLXQnNbwj
1010
github.com/G-Node/libgin v0.3.0/go.mod h1:VjulCBq7k/kgf4Eabk2f4w9SDNowWhLnK+yZvy5Nppk=
1111
github.com/G-Node/libgin v0.3.2 h1:pbOIm+paF4VqfrTvSu4sPXmo2j5XSOnfxf137uzAHIs=
1212
github.com/G-Node/libgin v0.3.2/go.mod h1:/2l42QsLZTneVp2LjPoPOWhVjvfbO26atvcbvWA+axI=
13+
github.com/Masterminds/semver/v3 v3.1.0 h1:Y2lUDsFKVRSYGojLJ1yLxSXdMmMYTYls0rCvoqmMUQk=
1314
github.com/Masterminds/semver/v3 v3.1.0/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs=
1415
github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo=
1516
github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI=
@@ -152,7 +153,9 @@ github.com/issue9/identicon v1.0.1/go.mod h1:UKNVkUFI68RPz/RlLhsAr1aX6bBSaYEWRHV
152153
github.com/jaytaylor/html2text v0.0.0-20190408195923-01ec452cbe43 h1:jTkyeF7NZ5oIr0ESmcrpiDgAfoidCBF4F5kJhjtaRwE=
153154
github.com/jaytaylor/html2text v0.0.0-20190408195923-01ec452cbe43/go.mod h1:CVKlgaMiht+LXvHG173ujK6JUhZXKb2u/BQtjPDIvyk=
154155
github.com/jinzhu/gorm v1.9.11/go.mod h1:bu/pK8szGZ2puuErfU0RwyeNdsf3e6nCX/noXaVxkfw=
156+
github.com/jinzhu/gorm v1.9.12 h1:Drgk1clyWT9t9ERbzHza6Mj/8FY/CqMyVzOiHviMo6Q=
155157
github.com/jinzhu/gorm v1.9.12/go.mod h1:vhTjlKSJUTWNtcbQtrMBFCxy7eXTzeCAzfL5fBZT/Qs=
158+
github.com/jinzhu/inflection v1.0.0 h1:K317FqzuhWc8YvSVlFMCCUb36O/S9MCKRDI7QkRKD/E=
156159
github.com/jinzhu/inflection v1.0.0/go.mod h1:h+uFLlag+Qp1Va5pdKtLDYj+kHp5pxUVkryuEj+Srlc=
157160
github.com/jinzhu/now v1.0.1/go.mod h1:d3SSVoowX0Lcu0IBviAWJpolVfI5UJVZZ7cO71lE/z8=
158161
github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU=
@@ -292,6 +295,7 @@ github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81P
292295
github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
293296
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
294297
github.com/syndtr/goleveldb v1.0.0/go.mod h1:ZVVdQEZoIme9iO1Ch2Jdy24qqXrMMOU6lpPAyBWyWuQ=
298+
github.com/t-tiger/gorm-bulk-insert v1.3.0 h1:9k7BaVEhw/3fsvh6GTOBwJ2RXk3asc5xs5m6hwozq20=
295299
github.com/t-tiger/gorm-bulk-insert v1.3.0/go.mod h1:ruDlk8xDl+8sX4bA7PQuYly9YEb3pbp1eP2LCyeRrFY=
296300
github.com/unknwon/cae v1.0.0 h1:i39lOFaBXZxhGjQOy/RNbi8uzettCs6OQxpR0xXohGU=
297301
github.com/unknwon/cae v1.0.0/go.mod h1:QaSeRctcea9fK6piJpAMCCPKxzJ01+xFcr2k1m3WRPU=

‎internal/app/api.go

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright 2020 The Gogs Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package app
6+
7+
import (
8+
"net/http"
9+
10+
"github.com/microcosm-cc/bluemonday"
11+
"gopkg.in/macaron.v1"
12+
13+
"github.com/G-Node/gogs/internal/context"
14+
)
15+
16+
func ipynbSanitizer() *bluemonday.Policy {
17+
p := bluemonday.UGCPolicy()
18+
p.AllowAttrs("class", "data-prompt-number").OnElements("div")
19+
p.AllowAttrs("class").OnElements("img")
20+
p.AllowURLSchemes("data")
21+
return p
22+
}
23+
24+
func SanitizeIpynb() macaron.Handler {
25+
p := ipynbSanitizer()
26+
27+
return func(c *context.Context) {
28+
html, err := c.Req.Body().String()
29+
if err != nil {
30+
c.Error(err, "read body")
31+
return
32+
}
33+
34+
c.PlainText(http.StatusOK, p.Sanitize(html))
35+
}
36+
}

‎internal/app/api_test.go

+95
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// Copyright 2020 The Gogs Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package app
6+
7+
import (
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
)
12+
13+
func Test_ipynbSanitizer(t *testing.T) {
14+
p := ipynbSanitizer()
15+
16+
tests := []struct {
17+
name string
18+
input string
19+
want string
20+
}{
21+
{
22+
name: "allow 'class' and 'data-prompt-number' attributes",
23+
input: `
24+
<div class="nb-notebook">
25+
<div class="nb-worksheet">
26+
<div class="nb-cell nb-markdown-cell">Hello world</div>
27+
<div class="nb-cell nb-code-cell">
28+
<div class="nb-input" data-prompt-number="4">
29+
</div>
30+
</div>
31+
</div>
32+
</div>
33+
`,
34+
want: `
35+
<div class="nb-notebook">
36+
<div class="nb-worksheet">
37+
<div class="nb-cell nb-markdown-cell">Hello world</div>
38+
<div class="nb-cell nb-code-cell">
39+
<div class="nb-input" data-prompt-number="4">
40+
</div>
41+
</div>
42+
</div>
43+
</div>
44+
`,
45+
},
46+
{
47+
name: "allow base64 encoded images",
48+
input: `
49+
<div class="nb-output" data-prompt-number="4">
50+
<img class="nb-image-output" src="data:image/png;base64,iVBORw0KGgoA"/>
51+
</div>
52+
`,
53+
want: `
54+
<div class="nb-output" data-prompt-number="4">
55+
<img class="nb-image-output" src="data:image/png;base64,iVBORw0KGgoA"/>
56+
</div>
57+
`,
58+
},
59+
{
60+
name: "prevent XSS",
61+
input: `
62+
<div class="nb-output" data-prompt-number="10">
63+
<div class="nb-html-output">
64+
<style>
65+
.output {
66+
align-items: center;
67+
background: #00ff00;
68+
}
69+
</style>
70+
<script>
71+
function test() {
72+
alert("test");
73+
}
74+
75+
$(document).ready(test);
76+
</script>
77+
</div>
78+
</div>
79+
`,
80+
want: `
81+
<div class="nb-output" data-prompt-number="10">
82+
<div class="nb-html-output">
83+
84+
85+
</div>
86+
</div>
87+
`,
88+
},
89+
}
90+
for _, test := range tests {
91+
t.Run(test.name, func(t *testing.T) {
92+
assert.Equal(t, test.want, p.Sanitize(test.input))
93+
})
94+
}
95+
}

‎internal/app/metrics.go

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright 2020 The Gogs Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package app
6+
7+
import (
8+
"net/http"
9+
10+
"gopkg.in/macaron.v1"
11+
12+
"github.com/G-Node/gogs/internal/conf"
13+
"github.com/G-Node/gogs/internal/context"
14+
)
15+
16+
func MetricsFilter() macaron.Handler {
17+
return func(c *context.Context) {
18+
if !conf.Prometheus.Enabled {
19+
c.Status(http.StatusNotFound)
20+
return
21+
}
22+
23+
if !conf.Prometheus.EnableBasicAuth {
24+
return
25+
}
26+
27+
c.RequireBasicAuth(conf.Prometheus.BasicAuthUsername, conf.Prometheus.BasicAuthPassword)
28+
}
29+
}

‎internal/assets/public/public_gen.go

+24-24
Large diffs are not rendered by default.

‎internal/assets/templates/templates_gen.go

+7-7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/cmd/web.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"gopkg.in/macaron.v1"
3030
log "unknwon.dev/clog/v2"
3131

32+
"github.com/G-Node/gogs/internal/app"
3233
"github.com/G-Node/gogs/internal/assets/public"
3334
"github.com/G-Node/gogs/internal/assets/templates"
3435
"github.com/G-Node/gogs/internal/conf"
@@ -688,16 +689,15 @@ func runWeb(c *cli.Context) error {
688689
apiv1.RegisterRoutes(m)
689690
}, ignSignIn)
690691

692+
// ***************************
693+
// ----- Internal routes -----
694+
// ***************************
691695
m.Group("/-", func() {
692-
if conf.Prometheus.Enabled {
693-
m.Get("/metrics", func(c *context.Context) {
694-
if !conf.Prometheus.EnableBasicAuth {
695-
return
696-
}
696+
m.Get("/metrics", app.MetricsFilter(), promhttp.Handler()) // "/-/metrics"
697697

698-
c.RequireBasicAuth(conf.Prometheus.BasicAuthUsername, conf.Prometheus.BasicAuthPassword)
699-
}, promhttp.Handler())
700-
}
698+
m.Group("/api", func() {
699+
m.Post("/sanitize_ipynb", app.SanitizeIpynb()) // "/-/api/sanitize_ipynb"
700+
})
701701
})
702702

703703
// robots.txt

‎internal/db/user.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ func NewGhostUser() *User {
534534
}
535535

536536
var (
537-
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", ".", ".."}
537+
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", ".", ".."}
538538
reservedUserPatterns = []string{"*.keys"}
539539
)
540540

‎public/plugins/marked-0.3.6/marked.min.js

-6
This file was deleted.

‎public/plugins/marked-0.8.1/marked.min.js

+6
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎public/plugins/notebookjs-0.3.0/notebook.min.js

-1
This file was deleted.

0 commit comments

Comments
 (0)