CSS for disabled styles can be accessed by everyone

[Kirby]

Affected version

2.2.10 PL 1

When delivering CSS via css.php, XenForo does not check if the style is enabled (or if the accessing user has permission to use disabled styles) and thus also delivers CSS of disabled styles to everyone.

This can be problematic if disabled styles produce errors when rendering (for example if they call templater functions that do not exist) as this would get logged as errors - which can happen quite often if sucher URLs are accessed by crawlers.

Ideally XenForo should return a 403 for CSS of a disabled style if the user does not have permission to use the style.

 

[Xon]

There are a couple issues at play:

• css.php deliberately run in the context that it is always guest access, thus allowing aggressive caching to be shared for all users
• The "user selectable" flag on a style is not actually a "disabled" flag.
• css.php arguments are not fully signed to prevent tampering.
  • There hash passed in is about the template list, and doesn't include the style & language ids.

 

[Kirby]

css.php deliberately run in the context that it is always guest access, thus allowing aggressive caching to be shared for all users

That's fine as long as the style is user selectable; if it is not XenForo should check if the user requesing the CSS can access it.

The "user selectable" flag on a style is not actually a "disabled" flag.

Well ... it's labeled as such

css.php arguments are not fully signed to prevent tampering.

Hmm, yeah - for the specific usecase that triggered this bug report ("Disabled" UI.X Styles & Add-on causing exceptions for undefined templater functions when a client was requesting CSS for such a style) that wouldn't really matter,

 

[Xon]

That's fine as long as the style is user selectable; if it is not XenForo should check if the user requesing the CSS can access it.

css.php doesn't load any session state, and really shouldn't ever touch per-user state. Everything in css.php needs to run as guest, with the assumption CSS is sharable cached resources.

Hmm, yeah - for the specific usecase that triggered this bug report ("Disabled" UI.X Styles & Add-on causing exceptions for undefined templater functions when a client was requesting CSS for such a style) that wouldn't really matter,

I feel the better fix for this particular issue is for styles to be able to depend on add-on(s) and refuse to render if those add-ons are not enabled.