1431539 - :-moz-any breaks the :visited invariants.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Emilio Cobos Álvarez [:emilio]]

The way it's implemented in stylo allows for :-moz-any selectors to the left of a non-link element to affect its style.

Filing as sec-sensitive just in case, though I think it isn't a security issue.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Testcase

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Clearer testcase

(The element should be green, it's red on stylo)

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Fix @ https://github.com/servo/servo/pull/19817

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

Looks like it isn't leaking the computed value to content script, so it's probably not a security issue.

If it does leak that, it may be a sec-moderate bug, though.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

(In reply to Xidorn Quan [:xidorn] UTC+10 (PTO Jan 19 ~ 29) from comment #4)
> Looks like it isn't leaking the computed value to content script, so it's
> probably not a security issue.
> 
> If it does leak that, it may be a sec-moderate bug, though.

Yeah it does not. Indeed the only way to cause this failure is with nested links, which triggers effectively the construction of a visited and unvisited style down the tree.

This is only the :visited style of an element getting affected by a + combinator, which breaks the model of the "closest relevant link", but I don't think it has any kind of implication, you still match both as visited and unvisited on both so you wouldn't be able to construct a timing attack either.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: Tests

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 8944362 [details] [diff] [review]
Tests

Review of attachment 8944362 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM!

Comment 8

[Emilio Cobos Álvarez [:emilio]]

I'm pretty sure this is not a security issue at this point, but I can't remove the flag :)

Comment 9

[Raul Gurzau (:RaulG)]

Backed out changeset de52bf2201a6 (bug 1431539) for mochitest failures on layout/style/test/test_visited_reftests.html on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/autoland/rev/7d6828a38971e2c0925ce89e5f906a47db06bb2b

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=de52bf2201a6992df26d0e7c7c1223266bcda4f9

Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=157777143&repo=autoland&lineNumber=16747

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Well, I tested the test failed without the patch, but not that it passed with it, whoops. Apparently visited-page.html isn't visited on the references, I'll just adapt the test-case a bit.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2bd97ba1cb81
Tests. r=dholbert

Comment 12

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/2bd97ba1cb81