Refuse a page that sets a cookie, reaches storage or carries a handler [#50]

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,

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)<meta\b[^>]*\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
+}

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+`<input type="search" />`, 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, `</head>`,
+			`  <meta http-equiv="set-cookie" content="seen=1" />`+"\n  </head>", 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+`<button onclick="alert(1)">Somewhere</button>`, 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, "  </head>",
+		`    <meta http-equiv="set-cookie" content="seen=1" />`+"\n  </head>", 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, "      <h1>{{ .Title }}</h1>",
+		"      <h1>{{ .Title }}</h1>\n      <script>localStorage.setItem(\"seen\", \"1\")</script>", 1))
+
+	var log bytes.Buffer
+	if err := Run(root, &log); err == nil {
+		t.Fatalf("Run passed a page naming a storage interface:\n%s", log.String())
+	}
+	if !strings.Contains(log.String(), "names localStorage") {
+		t.Errorf("the run does not name what it found; it said:\n%s", log.String())
+	}
+}
+
+// The one-character version of the mistake this row exists for. The row about a
+// script element reads the src attribute, so a page with no source anywhere and
+// one handler on one element runs code in a reader's browser and passes every
+// row this gate had before this one.
+func TestRunRefusesAPageCarryingAHandlerRatherThanAScriptSource(t *testing.T) {
+	handler := `<a href="/" onclick="alert(1)">Somewhere</a>`
+	root := tree(t, strings.Replace(goodTemplate, "      <h1>{{ .Title }}</h1>",
+		"      <h1>{{ .Title }}</h1>\n      "+handler, 1))
+
+	var log bytes.Buffer
+	if err := Run(root, &log); err == nil {
+		t.Fatalf("Run passed a page carrying a handler:\n%s", log.String())
+	}
+	if !strings.Contains(log.String(), "page-fetches-no-script: ok") {
+		t.Errorf("the row about a script source judged this page rather than passing it; it said:\n%s", log.String())
+	}
+	for _, want := range []string{
+		"page-carries-no-inline-handler: REFUSED, 2 violation(s)",
+		"the a element carries the handler attribute onclick",
+	} {
+		if !strings.Contains(log.String(), want) {
+			t.Errorf("the run does not carry %q; it said:\n%s", want, log.String())
+		}
+	}
+}
+
+// An address whose scheme runs code rather than fetching anything. It carries no
+// attribute name a handler pattern would find, and the row that reads what a
+// page references reads the host, which an address of this shape does not have.
+func TestRunRefusesAnAddressWhoseSchemeIsAScript(t *testing.T) {
+	root := tree(t, strings.Replace(goodTemplate, `<p><a href="/legal/">Who publishes this site</a></p>`,
+		`<p><a href="/legal/">Who publishes this site</a></p>`+
+			"\n      "+`<p><a href="javascript:alert(1)">Somewhere</a></p>`, 1))
+
+	var log bytes.Buffer
+	if err := Run(root, &log); err == nil {
+		t.Fatalf("Run passed a page whose address is a script:\n%s", log.String())
+	}
+	if !strings.Contains(log.String(), "carries an address whose scheme is a script") {
+		t.Errorf("the run does not say what it found; it said:\n%s", log.String())
+	}
+}
+
+// The neighbour that has to stay green, because both rows read names over the
+// whole document rather than inside anything. A page carrying none of what
+// either judges is passed by both, so a red run over the cases above is about
+// what was put on the page and not about the pages themselves.
+func TestTheTwoBrowserRowsPassAPageThatCarriesNeither(t *testing.T) {
+	var log bytes.Buffer
+	if err := Run(tree(t, goodTemplate), &log); err != nil {
+		t.Fatalf("Run refused a tree carrying neither: %v\n%s", err, log.String())
+	}
+	for _, want := range []string{
+		"page-touches-no-browser-storage: ok",
+		"page-carries-no-inline-handler: ok",
+	} {
+		if !strings.Contains(log.String(), want) {
+			t.Errorf("the run does not report %q; it said:\n%s", want, log.String())
+		}
+	}
+}