From 761bb3cf53960485921ad045bae5a79340d66f97 Mon Sep 17 00:00:00 2001 From: Unknwon Date: Fri, 31 Mar 2017 16:19:10 -0400 Subject: [PATCH] modules/markup: protect sanitizer from possible modification Only expose public APIs for 'Sanitize' and 'SanitizeBytes' to eliminate unintentional modifications to sanitizer policy. Also use 'sync.Once' to make sure multiple calls of 'NewSanitizer' is safe (although should never happen, but this is a better way). --- modules/markup/markdown.go | 19 +---------- modules/markup/markdown_test.go | 9 +++--- modules/markup/sanitizer.go | 55 ++++++++++++++++++++++++++++++++ modules/markup/sanitizer_test.go | 38 ++++++++++++++++++++++ modules/template/template.go | 2 +- routers/install.go | 2 +- 6 files changed, 101 insertions(+), 24 deletions(-) create mode 100644 modules/markup/sanitizer.go create mode 100644 modules/markup/sanitizer_test.go diff --git a/modules/markup/markdown.go b/modules/markup/markdown.go index fa91553ab..51afe48ec 100644 --- a/modules/markup/markdown.go +++ b/modules/markup/markdown.go @@ -14,7 +14,6 @@ import ( "strings" "github.com/Unknwon/com" - "github.com/microcosm-cc/bluemonday" "github.com/russross/blackfriday" "golang.org/x/net/html" @@ -27,22 +26,6 @@ const ( ISSUE_NAME_STYLE_ALPHANUMERIC = "alphanumeric" ) -var Sanitizer = bluemonday.UGCPolicy() - -// BuildSanitizer initializes sanitizer with allowed attributes based on settings. -// This function should only be called once during entire application lifecycle. -func BuildSanitizer() { - // We only want to allow HighlightJS specific classes for code blocks - Sanitizer.AllowAttrs("class").Matching(regexp.MustCompile(`^language-\w+`)).OnElements("code") - - // Checkboxes - Sanitizer.AllowAttrs("type").Matching(regexp.MustCompile(`^checkbox$`)).OnElements("input") - Sanitizer.AllowAttrs("checked", "disabled").OnElements("input") - - // Custom URL-Schemes - Sanitizer.AllowURLSchemes(setting.Markdown.CustomURLSchemes...) -} - var validLinksPattern = regexp.MustCompile(`^[a-z][\w-]+://|^mailto:`) // isLink reports whether link fits valid format. @@ -480,7 +463,7 @@ func Render(rawBytes []byte, urlPrefix string, metas map[string]string) []byte { urlPrefix = strings.Replace(urlPrefix, space, spaceEncoded, -1) result := RenderRaw(rawBytes, urlPrefix) result = PostProcess(result, urlPrefix, metas) - result = Sanitizer.SanitizeBytes(result) + result = SanitizeBytes(result) return result } diff --git a/modules/markup/markdown_test.go b/modules/markup/markdown_test.go index 2d06a149f..cbf753121 100644 --- a/modules/markup/markdown_test.go +++ b/modules/markup/markdown_test.go @@ -1,13 +1,14 @@ package markup_test import ( - . "github.com/gogits/gogs/modules/markup" - . "github.com/smartystreets/goconvey/convey" + "bytes" "testing" - "bytes" - "github.com/gogits/gogs/modules/setting" "github.com/russross/blackfriday" + . "github.com/smartystreets/goconvey/convey" + + . "github.com/gogits/gogs/modules/markup" + "github.com/gogits/gogs/modules/setting" ) func TestMarkdown(t *testing.T) { diff --git a/modules/markup/sanitizer.go b/modules/markup/sanitizer.go new file mode 100644 index 000000000..f685657bf --- /dev/null +++ b/modules/markup/sanitizer.go @@ -0,0 +1,55 @@ +// Copyright 2017 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 markup + +import ( + "regexp" + "sync" + + "github.com/microcosm-cc/bluemonday" + log "gopkg.in/clog.v1" + + "github.com/gogits/gogs/modules/setting" +) + +// Sanitizer is a protection wrapper of *bluemonday.Policy which does not allow +// any modification to the underlying policies once it's been created. +type Sanitizer struct { + policy *bluemonday.Policy + init sync.Once +} + +var sanitizer = &Sanitizer{} + +// NewSanitizer initializes sanitizer with allowed attributes based on settings. +// Multiple calls to this function will only create one instance of Sanitizer during +// entire application lifecycle. +func NewSanitizer() { + log.Trace("Markup: sanitizer initialization requested") + sanitizer.init.Do(func() { + sanitizer.policy = bluemonday.UGCPolicy() + // We only want to allow HighlightJS specific classes for code blocks + sanitizer.policy.AllowAttrs("class").Matching(regexp.MustCompile(`^language-\w+$`)).OnElements("code") + + // Checkboxes + sanitizer.policy.AllowAttrs("type").Matching(regexp.MustCompile(`^checkbox$`)).OnElements("input") + sanitizer.policy.AllowAttrs("checked", "disabled").OnElements("input") + + // Custom URL-Schemes + sanitizer.policy.AllowURLSchemes(setting.Markdown.CustomURLSchemes...) + + log.Trace("Markup: sanitizer initialized") + }) +} + +// Sanitize takes a string that contains a HTML fragment or document and applies policy whitelist. +func Sanitize(s string) string { + return sanitizer.policy.Sanitize(s) +} + +// SanitizeBytes takes a []byte slice that contains a HTML fragment or document and applies policy whitelist. +func SanitizeBytes(b []byte) []byte { + return sanitizer.policy.SanitizeBytes(b) +} diff --git a/modules/markup/sanitizer_test.go b/modules/markup/sanitizer_test.go new file mode 100644 index 000000000..a8f41fbf8 --- /dev/null +++ b/modules/markup/sanitizer_test.go @@ -0,0 +1,38 @@ +// Copyright 2017 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 markup_test + +import ( + "testing" + + . "github.com/smartystreets/goconvey/convey" + + . "github.com/gogits/gogs/modules/markup" +) + +func Test_Sanitizer(t *testing.T) { + BuildSanitizer() + Convey("Sanitize HTML string and bytes", t, func() { + testCases := []string{ + // Regular + `Google`, `Google`, + + // Code highlighting class + ``, ``, + ``, ``, + ``, ``, + + // Input checkbox + ``, ``, + ``, ``, + ``, ``, + } + + for i := 0; i < len(testCases); i += 2 { + So(Sanitize(testCases[i]), ShouldEqual, testCases[i+1]) + So(string(SanitizeBytes([]byte(testCases[i]))), ShouldEqual, testCases[i+1]) + } + }) +} diff --git a/modules/template/template.go b/modules/template/template.go index 0bd5fa3f1..faae266ba 100644 --- a/modules/template/template.go +++ b/modules/template/template.go @@ -125,7 +125,7 @@ func Safe(raw string) template.HTML { } func Str2html(raw string) template.HTML { - return template.HTML(markup.Sanitizer.Sanitize(raw)) + return template.HTML(markup.Sanitize(raw)) } func List(l *list.List) chan interface{} { diff --git a/routers/install.go b/routers/install.go index 2e6bee100..9d9bb6d18 100644 --- a/routers/install.go +++ b/routers/install.go @@ -62,7 +62,7 @@ func GlobalInit() { if setting.InstallLock { highlight.NewContext() - markup.BuildSanitizer() + markup.NewSanitizer() if err := models.NewEngine(); err != nil { log.Fatal(2, "Fail to initialize ORM engine: %v", err) }