Re: [csswg-drafts] [css-values] Security concerns regarding attr() (#5092)

The CSS Working Group just discussed `[css-values] Security concerns regarding attr()`, and agreed to the following:

* `RESOLVED change the spec such that violating attr security restriction will make it invalid at computed value time`
* `RESOLVED: change the spec such that violating attr security restriction will make it invalid at computed value time`

<details><summary>The full IRC log of that discussion</summary>
&lt;TabAtkins> on that note, I think the right behavior for scoped @property is that *the host element* will refuse to inherit from its parent, if its shadow registers a custom property.<br>
&lt;khush> moonira 2 open questions left for attr security. invalidation time and second is painting custom props<br>
&lt;TabAtkins> q+<br>
&lt;khush> for the first we propose making security violation. if we have a grammar that allows doing type, that doesn't cause security issues we can't say for sure<br>
&lt;khush> until after the substitution<br>
&lt;khush> in the current spec, attr is already a computed value time. there's no substition value<br>
&lt;khush> we just be consistent with security violation at computed value time<br>
&lt;khush> if attr is subsitutued into custom prop value. affected part of custom prop become tainted, not the whole custom prop value<br>
&lt;khush> moonira that's the proposal<br>
&lt;astearns> ack TabAtkins<br>
&lt;khush> TabAtkins i agree with both of these. url taining only causes invalid at computed value time, not parse time. makes it more diff to do that<br>
&lt;khush> indivisual tainiting is amazing! at the moment, spec says entire value is tainted if any part has attr. i assumed it's easier. if doing it for individual tokens is possible, which means we can combine sec and not security sensitive tokens in the same prop. that's great!<br>
&lt;khush> astearns 2 separate resolutions?<br>
&lt;emilio> q+<br>
&lt;astearns> ack emilio<br>
&lt;khush> emilio curious about how you do partial tainting?<br>
&lt;khush> q+<br>
&lt;khush> if that token gets used for a security sensitive spot then it's invalid, color is fine<br>
&lt;khush> q-<br>
&lt;khush> emilio feels sketchy<br>
&lt;khush> astearns we could resolve on what is proposed. and make it more strict?<br>
&lt;khush> TabAtkins presence of attr taints the entire custom prop which keeps chaining<br>
&lt;khush> emilio what is the computed value? does that contain attribute value?<br>
&lt;khush> TabAtkins computed value is post substitution<br>
&lt;khush> emilio i like the current spec more. i can see why it's more useful.<br>
&lt;khush> emilio feels unfortunate to track token tainiting lacking clear use-cases<br>
&lt;khush> TabAtkins both need to taint token, you need to tell that URL came from a tainted variable<br>
&lt;khush> emilio if i were to implement this, you would mark img as tainted.<br>
&lt;khush> the whole custom prop --img is tainted. rather than partial tainting<br>
&lt;khush> feels simpler to spec, reason about<br>
&lt;khush> TabAtkins on the spec side its only slightly simpler<br>
&lt;khush> implementation wise also feels same, you have to care about token ranges<br>
&lt;khush> one is example using bar and the other one is a URL<br>
&lt;chrishtr> q+<br>
&lt;khush> emilio very curious about use-case, feels more complex than current spec and unwarranted<br>
&lt;khush> iank_ i'll show you the chrome implementation<br>
&lt;iank_> https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/css_attr_value_tainting.h (i think)<br>
&lt;astearns> ack chrishtr<br>
&lt;iank_> moonira is that correct?<br>
&lt;khush> chrishtr the second part of the proposal, is the reason you suggested it is because it's more natural to implement<br>
&lt;khush> moonira yes, it's easier to implement that way<br>
&lt;khush> moonira it makes sense. the example in the issue, there it makes sense<br>
&lt;khush> moonira not sure<br>
&lt;khush> astearns what if we resolve on first part. and left the second part open so we could decide whether there is enough author justification<br>
&lt;emilio> https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/css_attr_value_tainting.cc;l=20;drc=f522344e45882da4c7f7cb1b3a0a7bd747d654bb seems pretty hacky to me fwiw, I'd rather keep a tainted bit on the value you substitute around, but I haven't thought it through<br>
&lt;khush> emilio +1<br>
&lt;khush> astearns PROPOSED RESOLUTION: change the spec such that violating attr security restriction will make it invalid at computed value time<br>
&lt;khush> astearns for now the tainting whole custom prop stays as is<br>
&lt;khush> can decide whether to make it partial later<br>
&lt;khush> RESOLVED change the spec such that violating attr security restriction will make it invalid at computed value time<br>
&lt;khush> RESOLVED: change the spec such that violating attr security restriction will make it invalid at computed value time<br>
</details>


-- 
GitHub Notification of comment by css-meeting-bot
Please view or discuss this issue at https://github.com/w3c/csswg-drafts/issues/5092#issuecomment-2378177905 using your GitHub account


-- 
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config

Received on Friday, 27 September 2024 00:30:32 UTC