Page MenuHomePhabricator

Application Security Review Request : ReaderExperiments
Closed, ResolvedPublic

Description

Project Information

───────────────────────────────────────────────────────────────────────────────
Language                 Files     Lines   Blanks  Comments     Code Complexity
───────────────────────────────────────────────────────────────────────────────
JavaScript                  21      2699      298       499     1902        193
JSON                        10       338        0         0      338          0
Vue                          9      1441      159       164     1118         49
Markdown                     5       390       87         0      303          0
License                      2       360       62         0      298          0
PHP                          2        63        8        24       31          3
TypeScript Typings           1        51       14        21       16          0
XML                          1         7        0         0        7          0
YAML                         1        20        0         0       20          0
───────────────────────────────────────────────────────────────────────────────
Total                       52      5369      628       708     4033        245
───────────────────────────────────────────────────────────────────────────────
Estimated Cost to Develop (organic) $116,816
Estimated Schedule Effort (organic) 6.08 months
Estimated People Required (organic) 1.71
───────────────────────────────────────────────────────────────────────────────
Processed 170212 bytes, 0.170 megabytes (SI)
───────────────────────────────────────────────────────────────────────────────

Description of the tool/project:
ReaderExperiments is the codebase being used by the newly-formed Reader Growth team for performing A/B tests of various reader-focused features live on production wikis. A general summary of this codebase (motivation behind it, architectural decisions, etc) can be found here; that document includes some info about security considerations for this project.

This codebase will be used to deploy a series of prototypes as A/B tests for logged-out users. Code will be hidden behind feature flags, and will not be shown to users unless they are 1) bucketed into a running A/B test, or 2) have explicitly opted in to see a prototype via a URL feature flag. Prototypes will be structured as individual resource modules, and they will primarily rely on client-side code to modify the reading experience in various ways; occasionally a small amount of PHP code may also be needed to support a prototype. All features in this codebase are considered temporary and disposable – if any feature tests well and is chosen to be scaled up for true production usage, it will need to be housed in a different codebase. See this ADR for more information about our approach here.

We expect prototypes to come and go over the lifespan of this extension, but the codebase itself will remain as a re-usable space where experiments can be conducted. These prototypes are reader-focused so they are concerned with retrieving / displaying existing data in novel ways; we are not going to be accepting new user-generated content or writing to the DB for our experiments as a general rule.

Description of how the tool will be used at WMF:
Currently we have a single prototype feature, Image Browsing, which adds a new UI to article pages for visual navigation through the page. We need to launch an A/B test of this feature before the end of October to satisfy the requirements of WE 3.1.3 in the WMF annual plan. We will rely on the new Experiment Platform (xLab) and the new edge unique cookies to perform an A/B test with this feature on logged-out users. The test will be performed on about half a dozen wikis, including English Wikipedia. The test will run for roughly 1 month and will then be switched off. Users who are not part of the test will see little or no impact (we are currently optimizing how we "gate" this feature as part of T404233; we will ensure that non-participating users are only impacted very slightly or not at all).

Dependencies
In addition to some existing JS libraries which are already included in MediaWiki (Codex, Vue.js), this project requires two additional JS libraries at runtime; these have been added to a local foreign-resources file:

These dependencies are specific to the ImageBrowsing feature, and can be removed when the prototype is no longer needed.

Has this project been reviewed before?
This is a new project that has not been reviewed before.

Working test environment
We are working on getting this extension to Beta as part of T404398 (we may deploy to beta even before a security review is complete if necessary). Until then, PatchDemo is the only way to stage this codebase. Please contact @egardner for help setting up an instance if you need one.

Post-deployment
Reader Growth team will manage this extension (and the adding/removal of various prototypes within it) going forward.

Details

Risk Rating
Low

Event Timeline

egardner added a project: Reader Growth Team.
egardner moved this task from Incoming/Inbox to Radar on the Reader Growth Team board.
sbassett changed the task status from Open to In Progress.Oct 7 2025, 3:51 PM
sbassett assigned this task to mmartorana.
sbassett moved this task from Back Orders to In Progress on the secscrum board.
sbassett subscribed.

@egardner - We are targeting a mid-quarter turnaround of this review, sometime in November 2025. This should not block any deployments on your end if you have tighter timelines.

@egardner - We are targeting a mid-quarter turnaround of this review, sometime in November 2025. This should not block any deployments on your end if you have tighter timelines.

Great, thanks for the update!

In the mean time I've been doing some self-review work here: https://phabricator.wikimedia.org/T405993

We have received formal manager approval from @HSwan-WMF (see T405993#11257325) to proceed with deployment of this new extension to production prior to the completion of this review, and will move forward with deployment next week.

mmartorana moved this task from In Progress to Our Part Is Done on the secscrum board.

Security Review Summary - T404751 - 2025-12-15
Last commit reviewed: d77fc0e

Summary

Overall, this extension has a low attack surface. The code shows good security awareness, and the scanner findings are all false positives. There are no vulnerable packages, so the overall risk rating is low.

Vulnerable Packages - Production
none

Vulnerable Packages - Development
none

Outdated Packages
As reported via npm outdated:
low risk
(no explicit vulnerabilities reported, simply noting for completeness' sake.)

PackageCurrentWantedLatest (Remediation)
@babel/preset-env7.25.47.25.47.28.5
@wikimedia/codex2.3.22.3.22.3.3
@wikimedia/codex-design-tokens2.3.22.3.22.3.3
@wikimedia/codex-icons2.3.22.3.22.3.3
grunt-eslint24.3.024.3.026.0.0
vue3.5.133.5.133.5.25

As reported via composer outdated:
low risk
(no explicit vulnerabilities reported, simply noting for completeness' sake.)

PackageCurrentWantedLatest (Remediation)
mediawiki/phan-taint-check-plugin7.0.08.0.08.0.0
netresearch/jsonmapper4.5.05.0.05.0.0
phan/phan5.5.15.5.25.5.2
phpcsstandards/phpcsextra1.4.01.5.01.5.0
phpcsstandards/phpcsutils1.1.11.2.21.2.2
sabre/event5.1.76.0.16.0.1
squizlabs/php_codesniffer3.13.24.0.14.0.1
symfony/console7.4.08.0.18.0.1
symfony/string8.0.08.0.18.0.1

Static Analysis Findings
Risk:.

  1. snyk reported no findings low risk
  2. semgrep reported 2 false positives low risk
resources/ext.readerExperiments.imageBrowsing/components/DetailViewCaption.vue
❯❱ javascript.vue.security.audit.xss.templates.avoid-v-html.avoid-v-html
      Dynamically rendering arbitrary HTML on your website can be very dangerous because it can easily
      lead to XSS vulnerabilities. Only use HTML interpolation on trusted content and never on user-  
      provided content.                                                                               
      Details: https://sg.run/0QEw                                                                    
                                                                                                      
        8┆ <p
        9┆   v-if="caption"
       10┆   ref="captionTextElement"
       11┆   class="ib-detail-view-caption__text"
       12┆   :class="{ 'ib-detail-view-caption__collapsed': !isCaptionExpanded }"
       13┆   v-html="caption"
                                                                                                     
resources/ext.readerExperiments.imageBrowsing/components/VisualTableOfContentsItem.vue
❯❱ javascript.vue.security.audit.xss.templates.avoid-v-html.avoid-v-html
      Dynamically rendering arbitrary HTML on your website can be very dangerous because it can easily
      lead to XSS vulnerabilities. Only use HTML interpolation on trusted content and never on user-  
      provided content.                                                                               
      Details: https://sg.run/0QEw                                                                    
                                                                                                      
       26┆ <figcaption
       27┆   v-if="caption"
       28┆   ref="captionTextElement"
       29┆   v-html="caption"
  1. horusec reported false positives low risk
==================================================================================

Language: JavaScript
Severity: HIGH
Line: 77
Column: 32
SecurityTool: HorusecEngine
Confidence: HIGH
File: /Users/manfredi/Desktop/ReaderExperiments/tests/jest/DetailViewCaption.test.js
Code: const maliciousAlt = '<script>alert("xss")</script><img src=x onerror=alert(1)>';
RuleID: HS-JAVASCRIPT-16
Type: Vulnerability
ReferenceHash: 4567f3fa421f243d71bc373d2b480cb79ed8705c546abc41f2788c02968d9476
Details: (1/1) * Possible vulnerability detected: Alert statements should not be used
alert(...) as well as confirm(...) and prompt(...) can be useful for debugging during development, but in production mode this kind of pop-up could expose sensitive information to attackers, and should never be displayed. For more information checkout the CWE-489 (https://cwe.mitre.org/data/definitions/489.html) advisory.

==================================================================================

Language: JavaScript
Severity: HIGH
Line: 100
Column: 34
SecurityTool: HorusecEngine
Confidence: HIGH
File: /Users/manfredi/Desktop/ReaderExperiments/tests/jest/DetailViewCaption.test.js
Code: const maliciousLabel = '<script>alert("xss")</script><img src=x onerror=alert(1)>';
RuleID: HS-JAVASCRIPT-16
Type: Vulnerability
ReferenceHash: 47abe28b60a833a00e1ae6b47f3d97437ce78b5de6b547de70d4cc0da234a865
Details: (1/1) * Possible vulnerability detected: Alert statements should not be used
alert(...) as well as confirm(...) and prompt(...) can be useful for debugging during development, but in production mode this kind of pop-up could expose sensitive information to attackers, and should never be displayed. For more information checkout the CWE-489 (https://cwe.mitre.org/data/definitions/489.html) advisory.

==================================================================================

Language: JavaScript
Severity: HIGH
Line: 92
Column: 32
SecurityTool: HorusecEngine
Confidence: HIGH
File: /Users/manfredi/Desktop/ReaderExperiments/tests/jest/VisualTableOfContentsItem.test.js
Code: const maliciousAlt = '<script>alert("xss")</script><img src=x onerror=alert(1)>';
RuleID: HS-JAVASCRIPT-16
Type: Vulnerability
ReferenceHash: 57e3082370d9b0e03dc399a7c8d0f5340b859fc6388dc75d4f7c8f60492fb4a5
Details: (1/1) * Possible vulnerability detected: Alert statements should not be used
alert(...) as well as confirm(...) and prompt(...) can be useful for debugging during development, but in production mode this kind of pop-up could expose sensitive information to attackers, and should never be displayed. For more information checkout the CWE-489 (https://cwe.mitre.org/data/definitions/489.html) advisory.

==================================================================================

Language: JavaScript
Severity: LOW
Line: 49
Column: 5
SecurityTool: HorusecEngine
Confidence: HIGH
File: /Users/manfredi/Desktop/ReaderExperiments/tests/jest/useMediaSearchResults.test.js
Code: origin: '*'
RuleID: HS-JAVASCRIPT-19
Type: Vulnerability
ReferenceHash: d34b025664f11ebc3c78b4170c622a534af0ce01a81b9ef4a5dbb6242c69d533
Details: (1/1) * Possible vulnerability detected: Having a permissive Cross-Origin Resource Sharing policy
Same origin policy in browsers prevents, by default and for security-reasons, a javascript frontend to perform a cross-origin HTTP request to a resource that has a different origin (domain, protocol, or port) from its own. The requested target can append additional HTTP headers in response, called CORS, that act like directives for the browser and change the access control policy / relax the same origin policy. The Access-Control-Allow-Origin header should be set only for a trusted origin and for specific resources. For more information checkout the OWASP A6:2017 (https://owasp.org/www-project-top-ten/2017/A6_2017-Security_Misconfiguration.html) advisory.

==================================================================================

In this analysis, a total of 4 possible vulnerabilities were found and we classified them into:
Total of Vulnerability HIGH is: 3
Total of Vulnerability LOW is: 1

==================================================================================
  1. bearer reported false positives low risk
MEDIUM: Observable Timing Discrepancy [CWE-208]
https://docs.bearer.com/reference/rules/javascript_lang_observable_timing
To ignore this finding, run: bearer ignore add d345329511be2b3ec2fe40934a753057_0

File: resources/ext.readerExperiments.stickyHeaders.common/init.js:16

 16 	if ( hash && hash.slice( 0, 1 ) === '#' ) {

LOW: Leakage of information in logger message [CWE-532]
https://docs.bearer.com/reference/rules/javascript_lang_logger_leak
To ignore this finding, run: bearer ignore add 8bf0153c185788b60951f1aaa4c3b392_0

File: resources/lib/fast-average-color/index.js:345

 345         console.error(error);

LOW: Leakage of information in logger message [CWE-532]
https://docs.bearer.com/reference/rules/javascript_lang_logger_leak
To ignore this finding, run: bearer ignore add 8bf0153c185788b60951f1aaa4c3b392_1

File: resources/lib/fast-average-color/index.js:423

 423                 console.error(originalError);
=====================================

244 checks, 3 findings

Miscellanous Issues/Points of Discussion/Nits
low risk
gitleaks returned no results. low risk
gitsecrets returned no results. low risk