diff --git a/content/privacy.txt b/content/privacy.txt index b581d5f..96aa6c8 100644 --- a/content/privacy.txt +++ b/content/privacy.txt @@ -24,15 +24,25 @@ checked: Nothing a page this site produces reaches for comes from a domain this and no embedded player is told that somebody opened a page here. [output-references-no-domain-outside-the-allowlist] -promised: No cookie is set and nothing is written into either browser storage - area. [#50] +checked: No cookie is set and nothing is written into either browser storage + area. [page-touches-no-browser-storage] + +checked: No page here carries a handler written onto an element, so nothing on + a page runs in answer to what a reader does with it. + [page-carries-no-inline-handler] promised: There is no form, no field and no other route by which a reader can send anything from a page here. [#50] -promised: No page needs scripting to be read, which is a wider statement than - the first checked one: what is refused today is a page fetching a script, and - a handler written into the page itself is refused by nothing yet. [#50] +promised: No page needs scripting to be read, which is wider than either of the + two checked statements about scripting above. What is refused today is a page + fetching a script and a handler written onto an element, and a script element + carrying its code inside the page is refused by nothing yet. [#50] + +promised: What the checks above read is the names a page would have to spell. + A page reaching the same interface through a value none of those spellings + finds is refused by nothing, and what would see it is a browser loading every + produced page and reporting what it did. [#50] residual: Whatever host answers for these files sees each request, which is the address it came from, what was asked for and when. Every host sees that much, diff --git a/internal/invariant/invariant.go b/internal/invariant/invariant.go index 78971a8..ac96895 100644 --- a/internal/invariant/invariant.go +++ b/internal/invariant/invariant.go @@ -310,6 +310,20 @@ func Rules() []Rule { Refuses: "a produced page carrying a script element with a src attribute, wherever it points", decide: decideScriptSrc, }, + { + ID: "page-touches-no-browser-storage", + Subject: ProducedPages, + Reason: "the privacy page states that no cookie is set and that nothing is written into either browser storage area, and a reader can check none of that from the outside, so the statement is worth the check that refuses a page breaking it rather than the paragraph making it", + Refuses: "a produced page carrying a meta element that sets a cookie, or naming an interface that reads or writes a cookie, a browser storage area or a reporting beacon", + decide: decideBrowserStorage, + }, + { + ID: "page-carries-no-inline-handler", + Subject: ProducedPages, + Reason: "the row above about a script element reads the src attribute, so a page carrying no source and a handler written onto an element runs in a reader's browser and passes it, which is the shape a site with a zero-byte scripting budget stops noticing", + Refuses: "a produced page carrying an event handler attribute on an element, or an address whose scheme is a script", + decide: decideInlineHandler, + }, { ID: "image-carries-its-own-dimensions", Subject: ProducedPages, @@ -2035,3 +2049,85 @@ func decideMarkers(body []byte) []string { func lineOf(body []byte, at int) int { return bytes.Count(body[:at], []byte("\n")) + 1 } + +// What the two rows about a reader's browser read. +// +// The interface names are the vocabulary a page has to spell in order to reach +// a cookie, either storage area or a reporting call. They are matched over the +// produced bytes rather than inside a script element, because there is no +// script element to look inside: what a page here could carry is a handler on +// an element or a style declaration, and a name appearing anywhere in the +// document is the thing being refused either way. +// +// The bound is that this reads names and not behaviour. A page reaching the +// same interface through a value none of these spellings finds is refused by +// nothing here, and the headless leg is where that is seen. It is also why a +// produced page that merely mentions one of these words in a sentence is +// refused: separating a name from a mention needs a reading of the document +// that these rows do not make, and refusing the mention is the direction that +// fails closed. +var ( + setCookieMeta = regexp.MustCompile(`(?is)]*\bhttp-equiv\s*=\s*["']?\s*set-cookie`) + storageName = regexp.MustCompile(`(?i)\b(document\.cookie|localStorage|sessionStorage|indexedDB|openDatabase|navigator\.sendBeacon|navigator\.cookieEnabled)\b`) + // An address whose scheme runs code rather than fetching anything. The + // space is allowed because a browser reads one and a pattern that did + // not would miss the spelling somebody actually pastes. + scriptScheme = regexp.MustCompile(`(?is)\b(?:href|src|action|formaction)\s*=\s*["']?\s*javascript\s*:`) + // An event handler attribute. The name is the whole of what a browser + // needs to run it, so the pattern is the name rather than anything about + // the value. + handlerAttribute = regexp.MustCompile(`^on[a-z]+$`) +) + +// decideBrowserStorage refuses a produced page that reaches for a cookie, a +// browser storage area or a reporting call. +// +// A meta element setting a cookie is separated from the interface names because +// the two fail differently and the repair is not the same. The element is a +// header this site chose to write into the document, which a host serves +// verbatim and a reader's browser acts on with no script involved at all; the +// names are code that would have to run. A refusal naming only one of them +// would send the next person looking in the wrong half of the page. +func decideBrowserStorage(body []byte) []string { + var details []string + for _, loc := range setCookieMeta.FindAllIndex(body, -1) { + details = append(details, fmt.Sprintf( + "line %d carries a meta element that sets a cookie, which a browser acts on with nothing running on the page", + lineOf(body, loc[0]))) + } + for _, loc := range storageName.FindAllIndex(body, -1) { + details = append(details, fmt.Sprintf( + "line %d names %s, which reads or writes something this site says it leaves alone", + lineOf(body, loc[0]), string(body[loc[0]:loc[1]]))) + } + return details +} + +// decideInlineHandler refuses a produced page carrying code written onto an +// element or into an address. +// +// This is the near miss the row above it does not catch. The scripting budget +// is zero bytes and the row that reads a script element reads its src +// attribute, so a page with no source anywhere and one handler on one element +// is a page that runs code in a reader's browser and passes every row this gate +// had. It is also the mistake somebody actually makes: a template gains a +// button, the button gains an onclick, and nothing about either looks like +// fetching a script. +func decideInlineHandler(body []byte) []string { + var details []string + for _, e := range walk(body) { + for name := range e.attrs { + if handlerAttribute.MatchString(name) { + details = append(details, fmt.Sprintf( + "line %d: the %s element carries the handler attribute %s, which runs in a reader's browser", + e.line, e.name, name)) + } + } + } + for _, loc := range scriptScheme.FindAllIndex(body, -1) { + details = append(details, fmt.Sprintf( + "line %d carries an address whose scheme is a script rather than something to fetch", + lineOf(body, loc[0]))) + } + return details +} diff --git a/internal/invariant/invariant_test.go b/internal/invariant/invariant_test.go index 01dff66..72ee5a1 100644 --- a/internal/invariant/invariant_test.go +++ b/internal/invariant/invariant_test.go @@ -212,6 +212,17 @@ func TestEveryRowRefusesItsOwnViolationAndPassesTheNeighbour(t *testing.T) { // goes in it. "page-names-every-control": []byte(strings.Replace(cleanPage, contentOpen, contentOpen+``, 1)), + // A cookie written as a meta element, which needs nothing running + // on the page: a host serves the document and a browser acts on it. + // It is the one way a site whose scripting budget is zero bytes + // still sets one. + "page-touches-no-browser-storage": []byte(strings.Replace(cleanPage, ``, + ` `+"\n ", 1)), + // The handler that arrives with a control somebody added. It carries + // no source anywhere, so the row about a script element passes it, + // and it runs in a reader's browser all the same. + "page-carries-no-inline-handler": []byte(strings.Replace(cleanPage, contentOpen, + contentOpen+``, 1)), "tracked-text-names-no-tool": b64(t, "QSBub3RlIGFib3ZlLgpHZW5lcmF0ZWQgYnkgQ2hhdEdQVCBhbmQgbGVmdCBpbi4K"), // The version put back where it is convenient, which is what // somebody does who is adding a step and does not know the file @@ -1479,3 +1490,110 @@ func TestRunRefusesATreeThatLostTheClaimAboutTheClients(t *testing.T) { t.Errorf("the refusal reads %q, which does not say what the tree lost", err) } } + +// The mistake in the place it is actually made. A cookie written as a meta +// element needs nothing running on the page: a host serves the document and the +// browser acts on it, so it is the one way a site with a zero-byte scripting +// budget still sets one. The frame is one file, so it reds every page at once +// and the run names each of them. +func TestRunRefusesAFrameThatSetsACookie(t *testing.T) { + root := tree(t, strings.Replace(goodTemplate, " ", + ` `+"\n ", 1)) + + var log bytes.Buffer + if err := Run(root, &log); err == nil { + t.Fatalf("Run passed a frame that sets a cookie:\n%s", log.String()) + } + for _, want := range []string{ + "page-touches-no-browser-storage: REFUSED, 2 violation(s)", + "carries a meta element that sets a cookie", + } { + if !strings.Contains(log.String(), want) { + t.Errorf("the run does not carry %q; it said:\n%s", want, log.String()) + } + } +} + +// The other half of the same row, and it is a different repair. A name is code +// that would have to run; the element above is a header the browser acts on with +// nothing running. A refusal naming only one of the two sends the next person +// looking in the wrong half of the page. +// +// The fixture is the near miss rather than a page with the word typed into a +// sentence. A script element carrying its code inside the page has no src +// attribute, so the row about a script element passes it, and what it does is +// write into the storage area this site says it leaves alone. +func TestRunRefusesAPageThatNamesAStorageInterface(t *testing.T) { + root := tree(t, strings.Replace(goodTemplate, "