Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitation fix from Gogs #1461

Merged
merged 7 commits into from
Apr 13, 2017
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ func (repo *Repository) DescriptionHTML() template.HTML {
sanitize := func(s string) string {
return fmt.Sprintf(`<a href="%[1]s" target="_blank" rel="noopener">%[1]s</a>`, s)
}
return template.HTML(descPattern.ReplaceAllStringFunc(markdown.Sanitizer.Sanitize(repo.Description), sanitize))
return template.HTML(descPattern.ReplaceAllStringFunc(markdown.Sanitize(repo.Description), sanitize))
}

// LocalCopyPath returns the local repository copy path
Expand Down Expand Up @@ -844,8 +844,8 @@ func cleanUpMigrateGitConfig(configPath string) error {
// createDelegateHooks creates all the hooks scripts for the repo
func createDelegateHooks(repoPath string) (err error) {
var (
hookNames = []string{"pre-receive", "update", "post-receive"}
hookTpl = fmt.Sprintf("#!/usr/bin/env %s\ndata=$(cat)\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\necho \"${data}\" | \"${hook}\"\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType)
hookNames = []string{"pre-receive", "update", "post-receive"}
hookTpl = fmt.Sprintf("#!/usr/bin/env %s\ndata=$(cat)\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\necho \"${data}\" | \"${hook}\"\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType)
giteaHookTpls = []string{
fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' pre-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf),
fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' update $1 $2 $3\n", setting.ScriptType, setting.AppPath, setting.CustomConf),
Expand Down
4 changes: 2 additions & 2 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func (u *User) UpdateDiffViewStyle(style string) error {
func (u *User) AfterSet(colName string, _ xorm.Cell) {
switch colName {
case "full_name":
u.FullName = markdown.Sanitizer.Sanitize(u.FullName)
u.FullName = markdown.Sanitize(u.FullName)
case "created_unix":
u.Created = time.Unix(u.CreatedUnix, 0).Local()
case "updated_unix":
Expand Down Expand Up @@ -867,7 +867,7 @@ func updateUser(e Engine, u *User) error {
u.Website = base.TruncateString(u.Website, 255)
u.Description = base.TruncateString(u.Description, 255)

u.FullName = markdown.Sanitizer.Sanitize(u.FullName)
u.FullName = markdown.Sanitize(u.FullName)
_, err := e.Id(u.ID).AllCols().Update(u)
return err
}
Expand Down
21 changes: 1 addition & 20 deletions modules/markdown/markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"strings"

"github.com/Unknwon/com"
"github.com/microcosm-cc/bluemonday"
"github.com/russross/blackfriday"
"golang.org/x/net/html"

Expand All @@ -29,24 +28,6 @@ const (
IssueNameStyleAlphanumeric = "alphanumeric"
)

// Sanitizer markdown sanitizer
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() {
// Normal markdown-stuff
Sanitizer.AllowAttrs("class").Matching(regexp.MustCompile(`[\p{L}\p{N}\s\-_',:\[\]!\./\\\(\)&]*`)).OnElements("code", "div", "ul", "ol", "dl")

// Checkboxes
Sanitizer.AllowAttrs("type").Matching(regexp.MustCompile(`^checkbox$`)).OnElements("input")
Sanitizer.AllowAttrs("checked", "disabled").OnElements("input")
Sanitizer.AllowNoAttrs().OnElements("label")

// Custom URL-Schemes
Sanitizer.AllowURLSchemes(setting.Markdown.CustomURLSchemes...)
}

// IsMarkdownFile reports whether name looks like a Markdown file
// based on its extension.
func IsMarkdownFile(name string) bool {
Expand Down Expand Up @@ -708,7 +689,7 @@ func render(rawBytes []byte, urlPrefix string, metas map[string]string, isWikiMa
urlPrefix = strings.Replace(urlPrefix, " ", "+", -1)
result := RenderRaw(rawBytes, urlPrefix, isWikiMarkdown)
result = PostProcess(result, urlPrefix, metas, isWikiMarkdown)
result = Sanitizer.SanitizeBytes(result)
result = SanitizeBytes(result)
return result
}

Expand Down
67 changes: 67 additions & 0 deletions modules/markdown/sanitizer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright 2017 The Gitea Authors. All rights reserved.
// 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 markdown

import (
"regexp"
"sync"

"code.gitea.io/gitea/modules/log"

"github.com/microcosm-cc/bluemonday"

"code.gitea.io/gitea/modules/setting"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge the gitea internal packages.

)

// 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("Markdown: 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("Markdown: sanitizer initialized")
})
}

// Sanitize takes a string that contains a HTML fragment or document and applies policy whitelist.
func Sanitize(s string) string {
if sanitizer.policy == nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about

func init() {
NewSanitizer(0
}

?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't wanna initialize it unless we need to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this maybe a race problem.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It uses sync.Once so there's no race-condition that I can see...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

NewSanitizer()
}
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 {
if len(b) == 0 {
// nothing to sanitize
return b
}
if sanitizer.policy == nil {
NewSanitizer()
}
return sanitizer.policy.SanitizeBytes(b)
}
35 changes: 35 additions & 0 deletions modules/markdown/sanitizer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Copyright 2017 The Gitea Authors. All rights reserved.
// 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 markdown

import (
"testing"

"github.com/stretchr/testify/assert"
)

func Test_Sanitizer(t *testing.T) {
NewSanitizer()
testCases := []string{
// Regular
`<a onblur="alert(secret)" href="http://www.google.com">Google</a>`, `<a href="http://www.google.com" rel="nofollow">Google</a>`,

// Code highlighting class
`<code class="random string"></code>`, `<code></code>`,
`<code class="language-random ui tab active menu attached animating sidebar following bar center"></code>`, `<code></code>`,
`<code class="language-go"></code>`, `<code class="language-go"></code>`,

// Input checkbox
`<input type="hidden">`, ``,
`<input type="checkbox">`, `<input type="checkbox">`,
`<input checked disabled autofocus>`, `<input checked="" disabled="">`,
}

for i := 0; i < len(testCases); i += 2 {
assert.Equal(t, testCases[i+1], Sanitize(testCases[i]))
assert.Equal(t, testCases[i+1], string(SanitizeBytes([]byte(testCases[i]))))
}
}
2 changes: 1 addition & 1 deletion modules/templates/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func Safe(raw string) template.HTML {

// Str2html render Markdown text to HTML
func Str2html(raw string) template.HTML {
return template.HTML(markdown.Sanitizer.Sanitize(raw))
return template.HTML(markdown.Sanitize(raw))
}

// List traversings the list
Expand Down
2 changes: 1 addition & 1 deletion routers/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func GlobalInit() {

if setting.InstallLock {
highlight.NewContext()
markdown.BuildSanitizer()
markdown.NewSanitizer()
if err := models.NewEngine(); err != nil {
log.Fatal(4, "Failed to initialize ORM engine: %v", err)
}
Expand Down