Page MenuHomePhabricator

User CSS and some of the site CSS loaded on Special:Preferences
Closed, ResolvedPublic

Description

On https://de.wikipedia.org/wiki/Spezial:Einstellungen my user CSS (https://de.wikipedia.org/wiki/Benutzer:Schnark/vector.css) and some of the site CSS (https://de.wikipedia.org/wiki/MediaWiki:Group-editor.css) is loaded. As this was considered a security issue in T72672, this shouldn't happen again.

Event Timeline

Schnark created this task.Sep 12 2016, 9:55 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 12 2016, 9:55 AM

Hi,

I'm having trouble reproducing this issue. https://de.wikipedia.org/wiki/User:Bawolff/vector.css is not loading for me on Special:Preferences. Is there anything different or special about your account that might be different from mine that would cause this issue?

Strange. I updated my CSS, but on Spezial:Einstellungen the previous version is loaded.

The screenshot shows that the style node is there, but still has old content (starting with table.metadata), and also shows that my personal styles for Echo are applied.

Anomie added a subscriber: Anomie.Sep 12 2016, 3:15 PM

I managed to reproduce this locally. git blame blamed rMW3e7a50d5fdb9: OutputPage: Make ResourceLoader position exemption more generic.

As far as I can tell, it seems like what happened is that before that revision site.styles, user.styles, and noscript were always passed to $rlClient->setExemptStates() in OutputPage::getRlClient() and then manually loaded after filtering in OutputPage::buildExemptModules(). After that revision, they are only added to $rlClient->setExemptStates() if they would be loaded after the server-side filtering, meaning that if the server-side filtering excludes them they are able to be loaded as dependencies by RL on the client side.

dpatrick triaged this task as Low priority.Sep 13 2016, 8:37 PM

@Anomie Confirmed. This can only happen when javascript that already made its way (through approved path ways) to the client. This approved script can't itself be a user or site scripts, as those are safely excluded on the server. It must be coming from a core or extension module that is already loaded on the preferences page.

This seems odd but there are some good use cases for this. For example, VisualEditor has a run-time mw.loader.using().always() call that waits for the site and user module to complete so that they have an opportunity to register custom plugins or change configuration before everything gets set in.

In practice this is more about execution order than lazy-loading, since those modules load on all pages by default.

I suspected at first that maybe Echo is doing something similar (which, unlike VisualEditor, is loaded on the preferences page). And contrary to regular pages, on the preferences page such code wouldn't be riding along the existing fetch for the user module, it would cause it to begin loading in the first place.

However, turns our that's not the case. First, let's understand what these recent changes did in the context of this issue (user.styles module loading on preferences page).

  1. Before August 8 (80e5b160e) - Status quo since 2012.
    • State and loading are set in the same place (makeResourceLoaderLink) which generates the relevant code after filtering. It has always been possible to explicitly load the site or user module on a restricted page from JavaScript. It doesn't make sense to restrict this from a malicious perspective since it's trivially worked around. There are other reasons to restrict this, but more about that later.
  2. From August 8 (80e5b160e) to Aug 18 (3e7a50d5fdb9):
    • Regression: The handling of user.styles was before filtering. This actually caused it to load by default on all pages, including restricted ones like preferences.
  3. After Aug 18 (3e7a50d5fdb9)
    • An unrelated regression was fixed by making all style modules follow the same code path. This fixed the accidental loading of user.styles on restricted pages.
    • The loading of the user scripts module is correctly prevented, but the state was hardcoded for convenience (Oops). This caused the module to be stuck in state loading indefinitely and can't even be loaded by accident.

The reason user.styles is being loaded on the preferences page is as follows: mediawiki.js contains some tracking code to emit an internal event signalling that all initially loading modules have completed. This code enumerates all modules in state loading and registers a success and error callback. When all callbacks have been invoked, it emits mwLoadEnd.

This very code triggers a dependency resolution on the user module. While it does not proceed to load user scripts (since the state machine claims they are already on the way), it does "discover" that user.styles is a dependency but not yet being loaded, and as such fetches it on demand.

Fixed in https://gerrit.wikimedia.org/r/310485.

(Not in the public commit message.) After applying this commit, observe that user.styles no longer gets loaded. Verify through mw.loader.getState('user.styles'), or by checking the network panel in browser dev tools; without this patch there's an outgoing request to load.php?modules=user.styles.

Krinkle closed this task as Resolved.Nov 2 2016, 12:11 AM
Krinkle claimed this task.
Tgr added a subscriber: Tgr.Apr 17 2017, 8:26 AM

This was fixed half a year ago, we had (I think) two security releases since then, so this task can probably be made public?

This was fixed half a year ago, we had (I think) two security releases since then, so this task can probably be made public?

^ @Bawolff / @Krinkle?

Legoktm changed the visibility from "Custom Policy" to "Public (No Login Required)".Nov 21 2017, 4:58 PM