Note: My patch is now much more complete and awaiting feedback :)

I think the {wikiid}session are the ones to really focus on. At first glance i dont see meaningful attacks for the others (although i have no idea what PHP_ENGINE is, or why metawikisession would be set on enwiki). Although i suppose applying it to other cookies can't hurt, and better to be secure by default.

In T240884#5796687, @Joe wrote:

I think the main question to answer is "does it make sense to create a safe regex evaluation service?".
I think in a void the answer is "no". It could make sense to create a small C++ program wrapping the main re2 functionality and shell out to it from php.
On the other hand, we have to consider the wikimedia infrastructure for this and there are two counterpoints to be made:

• Is this a service we can only expect MediaWiki to call? If not, that's a point in favour of creating a separate service
• Shelling out for us works well by using a combination of firejail and cgroups creation that won't work well in the future with cgroups v2 and containerization
• Performance might not be extremely relevant

Now on the last point: this proposal seems to worry a lot about performance, but I see no performance requirement spelled out. Without more context, both the choice of shelling out vs and RPC service, and the proposal to use gRPC for said service seem to me like premature optimizations.
So my questions are:

• What is the 95th percentile of latency in validating all the constraint on an item when editing it?
• What is the average, median and max number of regexes we need to validate per item?

Without answering those questions, we would just make choices by principle, while I think we should have a more pragmatic approach.

Note that the archives do replace @ signs with "at" for whatever good that does. Personally I think ease of finding answers to old questions outweighs spam concerns. The type of spammers that mass crawl the internet, probably also just subscribe to all public mailing lists and don't respect robots.txt

That sounds like its working as it should (?)

Just as an aside, the dirt simple solution here would be to shell out to grep -p (or even just to php fed just the preg_match call) and rely on limit.sh to prevent undue resourse usage.

Hmm, $wgFlaggedRevsHandleIncludes looks to be 2 for ruwikinews, and Комментарии (ns 102) is not in $wgFlaggedNamespaces. Reading the docs:

"2 - (FR_INCLUDES_STABLE)",
"  For each template/file, check if a version of it was used when the page was reviewed and if the template/file itself has a stable version; use the newest those versions",
"NOTE: We may have templates that do not have stable version. Also, given situational inclusion of templates (e.g. parser functions selecting template X or Y based on date) there may also be no \"review time version\" revision ID for a template used on a page. In such cases, we select the current (unreviewed) revision. Likewise for files."

It would be nice if this breaking change were announced on the list like previous breaking changes, with a several month transition period so at least we can provide a seamless transition to our users.

Ok, I get it, the query tab of $wgDebugToolbar only shows up if $wgDebugLogFile is set to something

I don't get it, I changed nothing but this randomly started working :S. Oh well, sorry for the spam

It should be noted that queries also no longer show up in the $wgDebugLogFile file, which I usually use as a catch-all for all debugging channels.

Just to clarify this task - we are only talking about "official" domains, we are not talking about blindly accepting what is in the host header, right?

How does SameSite=lax work with credentialed CORS requests? That's the only issue i could possibly see with

Maybe it's just that Debian has adjusted the memory limit policies between versions ?
We should compare the memory and disk limits in /etc/ImageMagick-6/policy.xml

In T234907#5760432, @pmiazga wrote:

I just wanted to highlight that we need to take precautions while working on the Vector skin. Some popular 3rd party skins/extensions depend heavily on Vector. For example Hydra developed/used by Gamepedia has HydraSkin, and the SkinHydra class extends SkinVector ( source code: https://gitlab.com/hydrawiki/skins/hydra/blob/master/SkinHydra.php ), therefore any bigger changes to SkinVector might break their sites.
It looks like at least one 3rd party is reusing Vector skin for their custom solutions. Therefore we can expect that many other 3rd parties do the same.

In T208188#5760085, @DannyS712 wrote:

Question: Why is this being done in a separate table rather than as a user preference (even if it is set using a dedicated special page instead of via preferences)? I.e., why not have the new special page, after ensuring authentication, etc. save a preference?

As a side note, it's vital to investigate soon and quickly, because if the logs get garbage collected, we'll have no way to understand what went wrong.

This is intentional. It can be controlled by $wgAllowSiteCSSOnRestrictedPages

Just writing this down for reference as it was annoying to figure out, I think the memcached key in question is dewiki:abusefilter:throttle:290:user,page:ce12284db0992734fd982f4a3ddd1f5f2b5c5408 (aka dewiki:abusefilter:throttle:290:user,page:sha1( "404212:Wikipedia Diskussion:Schiedsgericht/Anfragen/Diskussionskomplexim Umfeld \"Anetta Kahane\"" ) )

I can also confirm if i compile from (source Version: ImageMagick 7.0.9-8 Q16 x86_64 2019-12-16) that identify works properly on this tiff file.

Hmm, I'm using the version from debian buster

Version: ImageMagick 6.9.10-23 Q16 x86_64 20190101 https://imagemagick.org
Copyright: © 1999-2019 ImageMagick Studio LLC
License: https://imagemagick.org/script/license.php
Features: Cipher DPC Modules OpenMP 
Delegates (built-in): bzlib djvu fftw fontconfig freetype heic jbig jng jp2 jpeg lcms lqr ltdl lzma openexr pangocairo png tiff webp wmf x xml zlib

In T185664#5742901, @DannyS712 wrote:

The task specifies that there are no security bugs - T234736 is a security bug I filed in October

CSP is moving to using the reports-to header. At some point it might make sense to make our CSP reporting use this style of report-to with the varnishkafka beacon instead of the current solution involving the api module.

What I see as the biggest "blocker" for this to be used on wmf infrastructure, would be the fact, that you can not preload separate files which define the same class names or functions (as they would conflict obviously). So, using multiple different versions of MediaWiki within the same scope of a single php-fpm is not possible if you want to preload all versions. You would either need to decide if you want to preload only one version of MediaWiki or neither of them. I think that's kind of impossible to decide and, at the same time, get any benefit of preloading at all?

Based on the description here, it sounds pretty impossible to use this with hetdeploy, so is this about third parties only?

If nothing else, the message used should be switched from apierror-unknownerror to apierror-unknownerror-nocode, although actually useful error would be nice.

btw, for reference, at least locally, the command that is timing out is:

In T240455#5739243, @Fae wrote:

As a small experiment I used ImageMagick to remove the alpha channel

convert norrie.tiff -alpha off output.tiff

on the example file. Uploading using the wizard gave

Unknown error: "$1".

So I'm not sure that preprocessing the batch with something like ImageMagick is an easy fix.

Looks good.

To me this seems a bit like the wrong approach. Shouldnt the you are blocked api response comtain all the relavent info about the block, including ip if that is relavent. It seems a bit fragile to require the client to know that for this type of block, users ip is relavent info.

I mean, email addresses are probably the most provate info we store, so the message seems fair to me. Maybe it should say something like e.g. email address & whatever else is included.

Fwiw its really convinent to me to have a tag that are security protected bugs are tagged with. It doesnt have to be named security but having a tag is nice.

The plan is to have a preference, where users can adjust their CSP header (T208188). We will not be adding wmflabs.org to the allow list by default though but users will be able to opt into it.

In T223010#5187869, @Umherirrender wrote:

What is the different to T149346?

Guess i can mentor this for GCI

I guess the biggest risk here is phising potential, whether directly or via an open redirect?

Also what happens if a tool gets whitelisted and then fails to keep up high standards? Do we break all the links?

In T155029#5424117, @DannyS712 wrote:

Assuming that https://www.mediawiki.org/wiki/Module:ExtensionJson is kept updated by bot, I can update the template to extract data from there. At https://www.mediawiki.org/wiki/Module:Extension/sandbox I'm rewriting the current template display in lua, which eventually will control the entire infobox rather than just providing specific pieces of data.

In T237873#5720940, @Mainframe98 wrote:

I've tried using DeprecatedGlobal, but that class is not suitable for globals using non-objects; for arrays, it'll result in a PHP warning: Cannot use object of type DeprecatedGlobal as array. I'm not sure what else could serve a clear deprecation warning. You could add a check to DatabaseUpdater::getOldGlobalUpdates that checks if any of the globals is something else than [], but that doesn't tell you what extension caused that warning.

In T237873#5720149, @Mainframe98 wrote:

Well, my reasoning was that with T223939: Type-hint all onLoadExtensionSchemaUpdate hook handler $updater parameter with DatabaseUpdater type (which would result in prohibiting passing null; therefore making the backwards compatibility fallback an un-executable branch of code), any of the known extensions use it as backwards compatibility only, its effective obsolescence in 1.17 and the fact that some globals lack documentation, we're effectively dealing with dead code here. I think we'd be hardpressed to find anything relying on these globals that is still compatible with MediaWiki 1.35.

Why no deprecation? Doesnt seem like there is an urgent need to remove or that its blocking anything. Even if code search says no results this was still a very directly public api, there could be extensions elsewhere using it that would benefit from deprecation warnings.

I'm not the person doing the review, but I did notice there is an XSS in this banner: https://en.wikipedia.org/w/index.php?title=Talk:RandomPageThatDoesntexist&action=submit&section=new&preloadtitle=%3Cdiv%20class%3Dfrb%3E%3Cspan%20class%3D%22%25AVERAGE%25%22%20data-foo%3D%22%26lt%3Bscript%26gt%3Balert(%27XSS%20on%3A%20%27%20%2B%20document.domain)%26lt%3B%2Fscript%26gt%3B%22%3E%3C%2Fspan%3E%3C%2Fdiv%3E&nosummary=true&banner=scervantes_B1920_0701_enWW_dsk_p1_lg_template_share_inbanner&force=1&preview%00=yes

Filters are stored in externalstorage, right? It would be very difficult to do that in a single query. But a maintenance script to find all that match a certain regex is certainly do-able (To find them, as said above, autofix is hard). If someone writes a maintenance script, then anyone with shell access can run it (e.g. make a request with Wikimedia-Site-requests ). I imagine, for who can see the answer, it would be anyone who in principle would have the rights to see the answer on wiki (I guess that means stewards?) and people who have signed developer related ndas.

So I guess the next question is, where to set the CSP headers. My guess would be in sub cluster_fe_deliver of text-frontend.inc.vcl.erb, but I'm really not sure if that is the correct place.

In T239562#5703878, @ToBeFree wrote:

Filtering per namespace, specifically the User namespace, may be helpful as well. This would allow us to identify userpage spambots. Should a separate issue be opened for "Allow filtering spam blacklist log entries by namespace", or should this issue be renamed to "Allow filtering spam blacklist log entries by URL and namespace"?

Modern edge, chrome, safari and firefox all support woff2, so that's probably why it gives the same results for both firefox and safari UAs . I imagine you'll get different results for opera mini, IE11 and maybe older safari.

At the time i was kind of thinking of trusted contribs, as being kind of like autoconfirmed but for gerrit

In T208188#5691074, @sbassett wrote:

CREATE TABLE /*_*/csp_sources (
	csp_user unsigned int NOT NULL,
	csp_url VARBINARY(255) NOT NULL, /* not csp_domain ; per Daniel's comment */
	csp_timestamp BINARY(14) NOT NULL
);

Not sure if it was discussed above, but from a security/privacy perspective, it probably makes sense to redact csp_url (or whatever it ends up being) from dumps, etc. so as not to expose those specific resources to potential attackers. In fact, it might make sense to go even further and redact the entire table since I'm not sure how much informational value it has other than to potential attackers.

An additional concern - ideally, a malicious script looking to either exfiltrate data [To be clear, fully preventing exfiltration is basically impossible, and out of scope. CSP will prevent the obvious methods though] or receive instructions from an external source, should not be able to simply adjust the allow list. Otherwise there wouldn't be much point to having a whitelist. To that end, I would strongly like that adding an entry to the allow list requires the user to re-authenticate, similar to changing an email. This is one of my main motivations for proposing a separate table, instead of just adding a preference.

In T135963#5610199, @SD0001 wrote:

Localhost URLs should be added to the CSP. Not being able to load javascript from a local server would severely impede development of user scripts and gadgets. Thanks.

When is it acceptable to load external scripts (Current de-facto: same as above, although I'd like to change it)

Regarding UI for populating the table, the options seem to be:

In T208188#4940422, @eranroz wrote:

@Bawolff I see no one have commented here in the last weeks. CSP is enabled for ~3 months and there are important privacy concerns with current situation (T207900#4846582).
Would you mind closing the RFC and breaking this to tasks based on what was agreed here?