fix: image and URL references refuse a /\ path, which browsers read as another host (WR-20)
safeImageURL and ValidURLRef refused only //, so a stored og_image of /\evil.example/x.png loaded a third-party image in the admin. Both now refuse /\ like models.validTarget already does.
This commit is contained in:
2
assets/dist/seo.js
vendored
2
assets/dist/seo.js
vendored
File diff suppressed because one or more lines are too long
@@ -2,9 +2,13 @@ import { el, emit, labelsFor, uid, uiLocaleOf, type Labels } from './labels'
|
||||
|
||||
export const IMAGE_TAG = 'golem15-seo-image'
|
||||
|
||||
/** Whether a stored image URL may be shown: a path or an http(s) URL. */
|
||||
/**
|
||||
* Whether a stored image URL may be shown: a path or an http(s) URL. A
|
||||
* path starting with // or /\ is refused: browsers read both as another
|
||||
* host (like classes.ValidURLRef).
|
||||
*/
|
||||
export function safeImageURL(url: string): boolean {
|
||||
return /^\/(?!\/)/.test(url) || /^https?:\/\//i.test(url)
|
||||
return /^\/(?![\/\\])/.test(url) || /^https?:\/\//i.test(url)
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -152,14 +152,15 @@ func decodeImage(_ context.Context, in pact.FormFieldValue) (pact.FormFieldDecod
|
||||
return pact.FormFieldDecoded{Value: s}, nil
|
||||
}
|
||||
|
||||
// ValidURLRef reports whether v is a path ("/x", not "//x") or an absolute
|
||||
// http(s) URL with a host, without spaces or control characters.
|
||||
// ValidURLRef reports whether v is a path ("/x", not "//x" or "/\x",
|
||||
// which browsers read as another host) or an absolute http(s) URL with a
|
||||
// host, without spaces or control characters.
|
||||
func ValidURLRef(v string) bool {
|
||||
if v == "" || strings.ContainsFunc(v, func(r rune) bool { return r <= ' ' || r == 0x7f }) {
|
||||
return false
|
||||
}
|
||||
if strings.HasPrefix(v, "/") {
|
||||
return !strings.HasPrefix(v, "//")
|
||||
return !strings.HasPrefix(v, "//") && !strings.HasPrefix(v, `/\`)
|
||||
}
|
||||
u, err := url.Parse(v)
|
||||
return err == nil && (u.Scheme == "http" || u.Scheme == "https") && u.Host != ""
|
||||
|
||||
30
classes/urlref_test.go
Normal file
30
classes/urlref_test.go
Normal file
@@ -0,0 +1,30 @@
|
||||
package classes
|
||||
|
||||
import "testing"
|
||||
|
||||
// TestValidURLRef: a path or an absolute http(s) URL; a path that browsers
|
||||
// read as another host (// or /\) is refused (WR-20).
|
||||
func TestValidURLRef(t *testing.T) {
|
||||
for v, want := range map[string]bool{
|
||||
"/media/a.png": true,
|
||||
"https://cdn.example/a.png": true,
|
||||
"http://cdn.example/a.png": true,
|
||||
"//evil.example/x.png": false,
|
||||
`/\evil.example/x.png`: false,
|
||||
"javascript:alert(1)": false,
|
||||
"data:image/png;base64,AA": false,
|
||||
"a.png": false,
|
||||
"/a b.png": false,
|
||||
"": false,
|
||||
} {
|
||||
if got := ValidURLRef(v); got != want {
|
||||
t.Errorf("ValidURLRef(%q) = %v, want %v", v, got, want)
|
||||
}
|
||||
}
|
||||
if !ValidImageURL("") {
|
||||
t.Error("ValidImageURL(\"\") = false, want true (no image)")
|
||||
}
|
||||
if ValidImageURL(`/\evil.example/x.png`) {
|
||||
t.Error(`ValidImageURL("/\evil.example/x.png") = true, want false`)
|
||||
}
|
||||
}
|
||||
18
tests/elements/image.test.ts
Normal file
18
tests/elements/image.test.ts
Normal file
@@ -0,0 +1,18 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { safeImageURL } from '../../assets/src/elements/image'
|
||||
|
||||
// E7 golem15-seo-image: which stored URLs the preview may load.
|
||||
|
||||
describe('E7 safeImageURL', () => {
|
||||
it('accepts a path and an http(s) URL', () => {
|
||||
for (const url of ['/media/a.png', 'https://cdn.example/a.png', 'HTTP://cdn.example/a.png']) {
|
||||
expect(safeImageURL(url), url).toBe(true)
|
||||
}
|
||||
})
|
||||
|
||||
it('refuses addresses a browser reads as another host or another scheme (WR-20)', () => {
|
||||
for (const url of ['//evil.example/x.png', '/\\evil.example/x.png', 'javascript:alert(1)', 'data:image/png;base64,AA', 'a.png', '']) {
|
||||
expect(safeImageURL(url), url).toBe(false)
|
||||
}
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user