I have a set of 'monster' templates at English Wikisource , cl-act-paragraph (and related) , which currently use a considerable amount of inline CSS, or parameter passing via various levels of template... What differs between the templates is typically trivial details such as left/right or the amount of indentation.

Simplifying this down to use 'templated' CSS might be simpler and easier to maintain ( portions were already re-worked to use Lua)., so the ability to have parameters to a common CSS core would be useful.

Let's look at an example. If this feature existed, it might look something like this:

div.box {
    border: 1px solid -mw-var( color );
}

{{#tag:templatestyles|| src=Box/styles.css | color={{{color}}} }}
<div class="box">{{{text}}}</div>

So then you could write wikitext like

{{box|color=blue|This is in a blue box!}}

That seems ok, until you try to use it more than once with different colors:

{{box|color=blue|This is in a blue box!}}
{{box|color=red|This is in a red box!}}

The generated HTML would look something like this:

<style>
div.box {
    border: 1px solid blue;
}
</style>
<div class="box">This is in a blue box!</div>
<style>
div.box {
    border: 1px solid red;
}
</style>
<div class="box">This is in a red box!</div>

That's not going to work. The rule in the second <style> tag will override the first and both boxes will be red. Deduplication, if it doesn't take into account the parameterization, might instead remove the second instance leaving both boxes blue.

CSS does provide a way around this in the CSS Custom Properties for Cascading Variables Module Level 1 (which is currently supported by about 87% of browsers)

<style>
div.box {
    border: 1px solid var(--color, green);
}
</style>
<div class="box">Green box</div>
<div class="box" style="--color:red">Red box</div>
<div class="box" style="--color:blue">Blue box</div>
<div style="--color:orange">
 <div class="box">Orange box</div>
 <div class="box">Also an orange box</div>
</div>

The trick would be to make sure that someone can't write something like this

<style>
div.box {
    border: 1px solid var(--color, green);
    background: var(--bg, white);
}
</style>
<div class="box">Green box</div>
<div class="box" style="--color:red">Red box</div>
<div class="box" style="--color:blue; --bg:purple">Blue box with a purple background</div>

because

<div class="box" style="--bg:url('http://evil.example/tracker.png')">Uh oh, privacy violation</div>

also works with that example. That particular example wouldn't work in MediaWiki because MW forbids "url(" in inline style attributes, but css-sanitizer and TemplateStyles shouldn't rely on that.

We might allow it for only some safe properties, versus the "allowed anywhere" model described in the standard. Although even that would rely on no browser deciding to extend the syntax of a safe property in a way that opens some security hole.

On an aside, this request was also made on T155813: Decide on storage and delivery method for TemplateStyles CSS (there may be other discussion related in that task; I just happened to be reading it because of the discussion about TemplateStyling Template:Documentation):

In T155813#3018991, @Tgr wrote:

Make the system flexible enough that editors are willing to replace most existing templates. IMO this means some sort of template parameter passing - there are way too many complex templates that take arguments for color, width, float direction etc. I guess colors could be kept as inline styles (they aren't really device-dependent), and the rest can be handled by classnames (and specifying width is a bad idea in the first place)? But it would be nice if there was an explicit way to pass settings to the CSS code. Since 1A/C has been excluded in the previous bullet point, that would imply 2C in which case the parser tag could be used with {{#tag}} to take parameters. How would it work with the CSS parsing though? (We want to parse when the page is saved, to have sane error handling.) So I guess this goal is infeasible no matter what.

I will note that CSS3 also apparently supports an attr() function as well.

https://developer.mozilla.org/en-US/docs/Web/CSS/attr

This coupled with tag: might be a way to generate 'safe' CSS stylesheets in the desired manner once browsers support it...

In T200632#6120519, @ShakespeareFan00 wrote:

I will note that CSS3 also apparently supports an attr() function as well.

attr only exists in the spec.

This task is not about attr() support though.

In T200632#4623300, @Anomie wrote:

That particular example wouldn't work in MediaWiki because MW forbids "url(" in inline style attributes, but css-sanitizer and TemplateStyles shouldn't rely on that.

Hmm. Shouldn't they though? I'm all in favour of reducing dependencies and applying layered security, but given what the architecture actually is currently, isn't the correct approach to handle it through MW's allowable parameter value filter and allow e:TemplateStyles to rely on that?

It's not necessarily that that is a particularly elegant solution for a greenfield implementation, but given the filtering has to happen when the wikitext page is saved I don't really see how any other approach is even feasible.

I've added a contrived test-case here:-
https://test.wikipedia.org/wiki/Template:Varparmtest/testcases

For when https://phabricator.wikimedia.org/T288201 is resolved and var() support is re-enabled for CSS on test wiki.

The CSS sanitizer doesn't currently recognise var() constructions as valid though...

In T200632#4623300, @Anomie wrote:

That particular example wouldn't work in MediaWiki because MW forbids "url(" in inline style attributes, but css-sanitizer and TemplateStyles shouldn't rely on that.

Why should url() construct should be both escaped in MW core and in TemplateStyles? (even three times if TemplateStyles should not rely on CSS-Sanitizer)

This would just work with CSS variables. In template code you can do this:

<div class="flexi-cols" style="--flexi-cols--min-width: {{{min-width|150px}}}; --flexi-cols--gap: {{{gap|1em}}};">
</div>

And in CSS for the template:

.flexi-cols {
	--flexi-cols--min-width: 250px;
	--flexi-cols--gap: 1em;
	
	display: grid;
	grid-template-columns: repeat( auto-fill, minmax(var(--flexi-cols--min-width), 1fr) );
	gap: var(--flexi-cols--gap);
}

CSS variables are already supported in HTML so this kind of already works... You just need to load CSS with a mw.loader or @import to make this work.

I have a test page here which works (as long as you load CSS):
https://pl.wikipedia.org/wiki/Wikipedysta:Nux/test_flexi_columns_and_VE
Loading in [[Special:Mypage/common.css ]]

@import url("/w/index.php?action=raw&ctype=text/css&title=Wikipedysta:Nux/flexi-columns.css");

Also works when loaded in gadgets:
https://pl.wikipedia.org/w/index.php?title=MediaWiki:Gadget-lang-on-top.css&oldid=70251021

So there are some workarounds, but making this work in Templatestyles would be better for the sake of maintainability and performance (not loading CSS that is not needed).

Yeah, I think that given the current support for CSS variables there is no need in doing any specific code in TemplateStyles itself, and this task can be resolved with checking/adding for CSS variable support. T288201: Re-enable var() in inline CSS was resolved after browsers fixed the problem, so now template editors can just put variables like style="--test" and use them in TemplateStyles. But someone needs to write a patch implementing CSS variable support in css-sanitizer like this:

--test: blue;
color: var( --test, #000 );

In T200632#4623300

That seems ok, until you try to use it more than once with different colors:

{{box|color=blue|This is in a blue box!}}
{{box|color=red|This is in a red box!}}

The generated HTML would look something like this:

<style>
div.box {
    border: 1px solid blue;
}
</style>
<div class="box">This is in a blue box!</div>
<style>
div.box {
    border: 1px solid red;
}
</style>
<div class="box">This is in a red box!</div>

That's not going to work. The rule in the second <style> tag will override the first and both boxes will be red. Deduplication, if it doesn't take into account the parameterization, might instead remove the second instance leaving both boxes blue.

---

CSS does provide a way around this in the CSS Custom Properties for Cascading Variables Module Level 1 (which is currently supported by about 87% of browsers)

<style>
div.box {
    border: 1px solid var(--color, green);
}
</style>
<div class="box">Green box</div>
<div class="box" style="--color:red">Red box</div>
<div class="box" style="--color:blue">Blue box</div>
<div style="--color:orange">
 <div class="box">Orange box</div>
 <div class="box">Also an orange box</div>
</div>

The trick would be to make sure that someone can't write something like this

<style>
div.box {
    border: 1px solid var(--color, green);
    background: var(--bg, white);
}
</style>
<div class="box">Green box</div>
<div class="box" style="--color:red">Red box</div>
<div class="box" style="--color:blue; --bg:purple">Blue box with a purple background</div>

because

<div class="box" style="--bg:url('http://evil.example/tracker.png')">Uh oh, privacy violation</div>

also works with that example. That particular example wouldn't work in MediaWiki because MW forbids "url(" in inline style attributes, but css-sanitizer and TemplateStyles shouldn't rely on that.

We might allow it for only some safe properties, versus the "allowed anywhere" model described in the standard. Although even that would rely on no browser deciding to extend the syntax of a safe property in a way that opens some security hole.

Your suggested solutions don't seem to take into account this issue.

I don’t see how that issue would be relevant? Passing URLs in --vars is still prohibited by MediaWiki itself (replaced by /* insecure input */), TemplateStyles doesn’t have to do anything here. (You could’ve just linked to the comment.)

In T200632#8824663, @stjn wrote:

I don’t see how that issue would be relevant? Passing URLs in --vars is still prohibited by MediaWiki itself (replaced by /* insecure input */), TemplateStyles doesn’t have to do anything here. (You could’ve just linked to the comment.)

That particular example wouldn't work in MediaWiki because MW forbids "url(" in inline style attributes, but css-sanitizer and TemplateStyles shouldn't rely on that.

I saw that, I don’t think that it is necessarily correct. But even if you decide to follow that advice, you still end up with a list of safe properties where var() can be allowed, which is better than what’s happening now where it is disallowed everywhere despite being able to be used in style="" attribute. (Also, this style of commenting is starting to get really condescending.)

Is it possible to quantify what we're worried about here?

url() can lead to lead to all sorts of nastiness unless controlled tightly, yes (but allowing either only Commons, or deferring to CORS, cf. the third-party resources task, would be a good idea). calc() could be fed pathological input that breaks browsers (cf. T208881). There's the Billion Laughs attack that the spec calls out and which UAs are supposed to prevent in any case. What else?

I have maybe ~2 use cases for url() (both involving images from Commons), and a boatload for widths, heights, margins, padding, color, etc. Dumb stuff that's unlikely to be exploitable. Lack of calc() I can probably work around with {{#expr: … }} or Lua. I just need some way to let end users say they want 5px of padding rather than .25em, and a silver border instead of a #000 one, without moving all of it out of TemplateStyles and into inline style attributes. In fact, I would even prefer using parameter-passing or a special Lua function (defineCSSCustomProperty()) to define the custom properties rather than magic --* property names in inline styles.

And I ask because looking at the task it looks like it's blocked by some fundamental and as-yet-unsolvable security issue, just without quantifying what that issue is. And if the problem is just that nobody has the time to do it then that's fair enough, but it's a completely different problem that requires completely different approaches to progress.

The CSS Custom Properties spec has been around since 2012, longer even than CSS Syntax Level 3 as a stand-alone module, so it's not obvious to me that anything implementing that (as css-sanitizer claims to do) would have any fundamental issues dealing with Custom Properties.

url() can lead to lead to all sorts of nastiness unless controlled tightly...

Personally, I doubt you'd be able to do anything useful by trying to use the url in template styles. In a practical sense. AFAIK you can't add template styles to login page etc (and css-keylogger is a very far fetched idea anyway). Plus, even if you manage to trick the automatic CSS check, you'll probably trigger a human reaction on the RC ;). Therefore, in practice, I doubt that the attacker would be able to get anything useful for himself. At worst, an attacker would be able to obtain several IP addresses, but no usable context.

That particular example wouldn't work in MediaWiki because MW forbids "url(" in inline style attributes, but css-sanitizer and TemplateStyles shouldn't rely on that.

I think some AbuseFilter could be added as another layer of security. This would allow WMF staff or admins to update such a filter for any strange attack vectors that might arise in the future. In particular, you could block the use of CSS variables for everyone except administrators (and/xor interface administrators).