microcosm-cc / bluemonday

bluemonday: a fast golang HTML sanitizer (inspired by the OWASP Java HTML Sanitizer) to scrub user generated content of XSS
https://github.com/microcosm-cc/bluemonday
BSD 3-Clause "New" or "Revised" License
3.2k stars 175 forks source link

Add AllowURLSchemeWithCustomPolicy() to allow external fine grain control over allowed URLs. #7

Closed dmitshur closed 10 years ago

dmitshur commented 10 years ago

Closes #6.

Feedback absolutely welcome, I am open for changes. Let me know what you think.

dmitshur commented 10 years ago

Typically, I would add tests, but in this case I'm not sure what kind of tests you'd want. Since the burden of implementing a custom url policy falls on the user, not bluemonday, it doesn't make sense to try to test url policies here.

However, perhaps a simple test that the policy is used correctly would be applicable.

For reference, I've used the following tests in my code that imports bluemonday, and they all pass with this PR:

func ExampleDataUriImage() {
    unsanitized := `<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==">`

    p := bluemonday.UGCPolicy()
    p.AllowURLSchemeWithCustomPolicy("data", dataUriImagePolicy)

    os.Stdout.WriteString(p.Sanitize(unsanitized))

    // Output:
    //<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==">
}

func ExampleDataUriNonImage() {
    unsanitized := `<img src="data:text/javascript;charset=utf-8,alert('hi');">`

    p := bluemonday.UGCPolicy()
    p.AllowURLSchemeWithCustomPolicy("data", dataUriImagePolicy)

    os.Stdout.WriteString(p.Sanitize(unsanitized))

    // Output:
}

func ExampleDataUriNonImage2() {
    unsanitized := `<img src="data:image/png;base64,charset=utf-8,alert('hi');">`

    p := bluemonday.UGCPolicy()
    p.AllowURLSchemeWithCustomPolicy("data", dataUriImagePolicy)

    os.Stdout.WriteString(p.Sanitize(unsanitized))

    // Output:
}

func ExampleDataUriNonImage3() {
    unsanitized := `<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAYAAACNbyblAAAAHElEQVQI12P4-_8/w38GIAXDIBKE0DHxgljNBAAO9TXL0Y4OHwAAAABJRU5ErkJggg==">`

    p := bluemonday.UGCPolicy()
    p.AllowURLSchemeWithCustomPolicy("data", dataUriImagePolicy)

    os.Stdout.WriteString(p.Sanitize(unsanitized))

    // Output:
}

func dataUriImagePolicy(url *url.URL) (allowUrl bool) {
    // TODO: Allow jpeg maybe?
    const prefix = "image/png;base64,"
    if !strings.HasPrefix(url.Opaque, prefix) {
        return false
    }
    if _, err := base64.StdEncoding.DecodeString(url.Opaque[len(prefix):]); err != nil {
        return false
    }
    if url.RawQuery != "" || url.Fragment != "" {
        return false
    }
    return true
}
$ go test . -v --run="DataUri"
=== RUN: ExampleDataUriImage
--- PASS: ExampleDataUriImage (422.744us)
=== RUN: ExampleDataUriNonImage
--- PASS: ExampleDataUriNonImage (229.402us)
=== RUN: ExampleDataUriNonImage2
--- PASS: ExampleDataUriNonImage2 (152.632us)
=== RUN: ExampleDataUriNonImage3
--- PASS: ExampleDataUriNonImage3 (166.02us)
grafana-dee commented 10 years ago

Hah, you beat me to it, I was packing and writing the other comment.

Accepted, I'll merge now.

dmitshur commented 10 years ago

Thanks a lot for merging! Makes me very happy. :grinning: I want to clear something up...

You've asked me to do some extra things in https://github.com/microcosm-cc/bluemonday/issues/6#issuecomment-48072347; do you still want me to make another PR with those things done?

grafana-dee commented 10 years ago

Nah, I'm on top of it, I've merged your tests into the suite, and am wrapping the example you gave into a helper that will allow data URI images.

dmitshur commented 10 years ago

Great! Thanks so much again!

Btw, have a great vacation!