In T181803#5666088, @Apap04 wrote:

Yeah, but you wouldn't want someone malicious to look at this issue and target WM's mailing list to get all those passwords and usernames out, right? I'm not saying that would happen since you need to have some sort of database access, but it could be worse if it was someone inside the Foundation.

Ok, backports done, CVE filed (CVE-2019-18987), tracking in T234983. I'm going to resolve this for now. Thanks everyone.

In T237887#5653650, @Daimona wrote:

Exactly, thanks! I just forgot one thing: adding a @fixme comment right above the if, saying something like:

@fixme Temporary stopgap for T237887

Just in case that code survives for more than expected.

Thanks, @Aklapper. I've gone ahead and added a new section here:

In T149869#5654801, @Yaron_Koren wrote:

It would be great if you could look at this fix and let me know if it does indeed solve the problem.... assuming you still do that sort of thing!

In T227209#5642888, @Arlolra wrote:

phan-taint-check-plugin@latest (either 2.1.0 or master) and mediawiki-phan-config@0.8.0 seem to have incompatible constraints in the version of phan required.
I'm going to leave that patch as WIP until that can be rectified but otherwise consider the above concerns addressed.

In T233213#5647214, @Addshore wrote:

@JBennett I think we can make this public now?
I had a quick flick through the comments and don't see anything private in here.

In T237887#5658015, @Urbanecm wrote:

Sure, I assume you mean 1) CVE 2) backports 3) tracking at T234983 and then making this task public?

Yes.

In T237887#5657534, @Urbanecm wrote:

19:18 <Urbanecm> !log Deploy security patch for T237887

@sbassett Could you do the final honours?

Some further discussions of this oft-reported issue: T14945, T40417.

Extremely low-level information disclosure of various configuration files are not typically considered vulnerabilities for FLOSS code such as MediaWiki, as said configuration files are publicly-available in various repositories. This is both known and intentional. Additionally, such files can often be deleted once MediaWiki has been installed and configured. A web server running MediaWiki can also be configured not to serve such files.

Extremely low-level information disclosure of various configuration files are not typically considered vulnerabilities for FLOSS code such as MediaWiki, as said configuration files are publicly-available in various repositories. This is both known and intentional. Additionally, such files can often be deleted once MediaWiki has been installed and configured. A web server running MediaWiki can also be configured not to serve such files.

Extremely low-level information disclosure of various configuration files are not typically considered vulnerabilities for FLOSS code such as MediaWiki, as said configuration files are publicly-available in various repositories. This is both known and intentional. Additionally, such files can often be deleted once MediaWiki has been installed and configured. A web server running MediaWiki can also be configured not to serve such files.

Extremely low-level information disclosure of various configuration files are not typically considered vulnerabilities for FLOSS code such as MediaWiki, as said configuration files are publicly-available in various repositories. This is both known and intentional. Additionally, such files can often be deleted once MediaWiki has been installed and configured. A web server running MediaWiki can also be configured not to serve such files.

Extremely low-level information disclosure of various configuration files are not typically considered vulnerabilities for FLOSS code such as MediaWiki, as said configuration files are publicly-available in various repositories. This is both known and intentional. Additionally, such files can often be deleted once MediaWiki has been installed and configured. A web server running MediaWiki can also be configured not to serve such files.

Unrelated to Wikimedia production sites.

Extremely low-level information disclosure of various configuration files are not typically considered vulnerabilities for FLOSS code such as MediaWiki, as said configuration files are publicly-available in various repositories. This is both known and intentional. Additionally, such files can often be deleted once MediaWiki has been installed and configured. A web server running MediaWiki can also be configured not to serve such files.

Extremely low-level information disclosure of various configuration files are not typically considered vulnerabilities for FLOSS code such as MediaWiki, as said configuration files are publicly-available in various repositories. This is both known and intentional. Additionally, such files can often be deleted once MediaWiki has been installed and configured. A web server running MediaWiki can also be configured not to serve such files.

Extremely low-level information disclosure of various configuration files are not typically considered vulnerabilities for FLOSS code such as MediaWiki, as said configuration files are publicly-available in various repositories. This is both known and intentional. Additionally, such files can often be deleted once MediaWiki has been installed and configured. A web server running MediaWiki can also be configured not to serve such files.

Extremely low-level information disclosure of various configuration files are not typically considered vulnerabilities for FLOSS code such as MediaWiki, as said configuration files are publicly-available in various repositories. This is both known and intentional. Additionally, such files can often be deleted once MediaWiki has been installed and configured. A web server running MediaWiki can also be configured not to serve such files.

The <ScRiPt%20>BL79(9647)</ScRiPt> XSS payload referenced within the output of the automated security scan above is returned as part of a JSON response from the MediaWiki action API with a content-type of application/json`. No modern web browsers should be rendering this content as HTML where such a naive XSS payload would be executed. Additionally, the output appears to be properly sanitized when the action API results are returned as HTML (format=html). Resolving as invalid.

The <ScRiPt%20>BL79(9647)</ScRiPt> XSS payload referenced within the output of the automated security scan above is returned as part of a JSON response from the MediaWiki action API with a content-type of application/json`. No modern web browsers should be rendering this content as HTML where such a naive XSS payload would be executed. Additionally, the output appears to be properly sanitized when the action API results are returned as HTML (format=html). Resolving as invalid.

The <ScRiPt%20>BL79(9647)</ScRiPt> XSS payload referenced within the output of the automated security scan above is returned as part of a JSON response from the MediaWiki action API with a content-type of application/json. No modern web browsers should be rendering this content as HTML where such a naive XSS payload would be executed. Additionally, the output appears to be properly sanitized when the action API results are returned as HTML (format=html). Resolving as invalid.

This is unrelated to any Wikimedia production website or project and is instead an issue with the website against which this scan was run: wikicod.ir. Resolving as invalid.

The output of the automated security scan pasted above appears to reference a JSONP-related vulnerability for the Java Spring framework. MediaWiki is written in PHP and does not use Java for any of its core components, so this is a false positive. While we appreciate anybody taking the time to investigate security-related issues within MediaWiki or any other Wikimedia code, filing Phabricator tasks for false positives like this, extremely low-level information disclosure vulnerabilities which are most likely invalid for FLOSS code, issues which have not been thoroughly investigated and provided without explicit steps for reproduction, or where an existing Phabricator task already exists is at best, very unhelpful. Additionally, please find and review our published steps for responsibly reporting security issues.

In T237449#5644361, @thcipriani wrote:

Can we update the wikimedia-security group to include @Dsharpe and @JFishback_WMF?

Done.

In T227346#5638509, @Mholloway wrote:

Thanks for digging in on that, @sbassett. I imagine the htmlentities call was originally included out of habit when constructing a URL, but in this case the $id values will only be valid Wikidata IDs (i.e., 'Q' followed by a positive integer) from the DB and thus would need no escaping (even if they weren't escaped anyway within Html::element). I've submitted a new patch to remove the htmlentities call and the Phan suppression.

In T227346#5636600, @sbassett wrote:

In T227346#5636495, @Mholloway wrote:

About that SecurityCheck-DoubleEscaped suppression, the rule is in fact triggered in CI if the suppression is removed:

Interesting. I think that may be a bug - I'll file one for the phan-taint-check-plugin project. But yes, it obviously needs to be suppressed for now.

In T227346#5636495, @Mholloway wrote:

About that SecurityCheck-DoubleEscaped suppression, the rule is in fact triggered in CI if the suppression is removed:

@WMDE-leszek Excellent, thanks.

In T237118#5630251, @MoritzMuehlenhoff wrote:

• "Tag {{former staff}} on any relevant project user profile pages" isn't done as part of the offboarding steps handled by SRE. If someone deals with that, then I'm not sure who :-)

Note: the above is a fairly basic review. I didn't focus on any performance-related issues as they pertain to security, as I'm hoping that's happening in T230813. I might also play around with the ext from a black box perspective next week with @Mholloway's docker environment. At a cursory review, r547555 and r547688 both look fine to me, but I might also review those a bit more next week. But none of this should block any deployments, etc.

Security Review Summary - T227346 - 2019-11-01
Overall, the MachineVision extension looks pretty good. There are a handful of issues below, but nothing I would categorize as above low for right now.

@WMDE-leszek - no problem, I was just trying to be proactive :)

As discussed per the meeting today (2019-10-19) and via @Ejegg's email, the Security-Team is comfortable with this change in functionality and would assign a low risk for this change. Please let us know if any further issues arrive and feel free to keep us apprised of any movement away from using iframes or similar elements within these payment contexts.

@Daimona - great, thanks for the help! Once the remaining backports are merged, I'll resolve this task.

In T226945#5614208, @Daimona wrote:

In T226945#5370566, @Legoktm wrote:

We absolutely need phan for release branches. Many of the regressions in previous releases could've been caught if phan were running.

This. It's just too easy to introduce regressions when backporting changes, especially big changes like security changes. Example: https://gerrit.wikimedia.org/r/#/q/Ie23e8234ae550273bf3f6f9c5ac45b7fc54eec2a this would've broken 1.3[123], but no tests failed.