Greetings! The issue arises from a vulnerability within yardoc. I've issued a GitHub Security Advisory concerning this, and contacted lsegal, the developer of yardoc, via email two days ago.

Loren just responded to my email, and he said he will check out my GHSA report.

It looks like this issue is due to some really unfortunate javascript code. If one views the html source of the XSS payload URL within the above description, you can see:

var match = unescape(window.location.hash).match(/^#!(.+)/);
var name = match ? match[1] : 'index.html';
name = name.replace(/^(\w+):\/\//, '').replace(/^\/\//, '');
window.top.location = name;

which is attempting to match anything after #! within the query string, perform some minor slash-escaping and then write it directly to the browser's location. Even implementing some basic url scheme sanitization as described here and other places would likely be mostly effective in mitigating this issue.

Yes, that's what I reported. Until a fix is implemented, I'd suggest removing the frames.html file because it serves no purpose.

My exact report was:

Summary

The "frames.html" file within the Yardoc's generated documentation is vulnerable to Cross-Site Scripting (XSS) attacks due to inadequate sanitization of user input within the JavaScript segment of the "frames.erb" template file.

Details

The vulnerability stems from mishandling user-controlled data retrieved from the URL hash in the embedded JavaScript code within the "frames.erb" template file. Specifically, the script lacks proper sanitization of the hash data before utilizing it to establish the top-level window's location. This oversight permits an attacker to inject malicious JavaScript payloads through carefully crafted URLs.

Snippet from "frames.erb":

erb
<script type="text/javascript">
  var match = unescape(window.location.hash).match(/^#!(.+)/);
  var name = match ? match[1] : '<%= url_for_main %>';
  name = name.replace(/^(\w+):\/\//, '').replace(/^\/\//, '');
  window.top.location = name;
</script>

PoC (Proof of Concept)

To exploit this vulnerability:

1. Gain access to the generated Yard Doc.
2. Locate and access the "frames.html" file.
3. Construct a URL containing the malicious payload in the hash segment, for instance: #!javascript:xss

Impact

This XSS vulnerability presents a substantial threat by enabling an attacker to execute arbitrary JavaScript code within the user's session context. Potential ramifications include session hijacking, theft of sensitive data, unauthorized access to user accounts, and defacement of websites. Any user visiting the compromised page is susceptible to exploitation. It is critical to promptly address this vulnerability to mitigate potential harm to users and preserve the application's integrity.

In T358507#9577616, @RedYetiDev wrote:

Yes, that's what I reported. Until a fix is implemented, I'd suggest removing the frames.html file because it serves no purpose.

Tagging Infrastructure-Foundations to see if there's a not-too-terrible way to do this via puppet config. Maybe just a rewrite rule from frames.html to index.html.

Sounds good!

We could also add a sandbox directive to the csp policy

In T358507#9577639, @Bawolff wrote:

We could also add a sandbox directive to the csp policy

Does yard just generate basic html/css? If so, then this would be fine, but if it creates any basic javascript functionality or interactions, those would break, I think. Unless we restrict the new CSP to just frames.html.

Yard just generates HTML/CSS/JS.

I suggest just removing frames.html, its not really used anywhere

By the way, this applies to several of WikiMedia's ruby packages:
https://doc.wikimedia.org/puppet/frames.html#!javascript:alert(%22XSS%22)
https://doc.wikimedia.org/rubygems/mediawiki-selenium/frames.html#!javascript:alert(%22XSS%22)
https://doc.wikimedia.org/mediawiki-vagrant/frames.html#!javascript:alert(%22XSS%22)
https://doc.wikimedia.org/rubygems/mediawiki-ruby-api/frames.html#!javascript:alert(%22XSS%22)

Good morning! Here is the latest update on patching this issue upstream.

1. My suggestion

--- a/templates/default/fulldoc/html/frames.erb
+++ b/templates/default/fulldoc/html/frames.erb
@@ -1,17 +1,27 @@
 <!DOCTYPE html>
 <html>
+
 <head>
   <meta charset="<%= charset %>">
-	<title><%= options.title %></title>
+  <title><%= options.title %></title>
 </head>
 <script type="text/javascript">
-  var match = unescape(window.location.hash).match(/^#!(.+)/);
-  var name = match ? match[1] : '<%= url_for_main %>';
-  name = name.replace(/^(\w+):\/\//, '').replace(/^\/\//, '');
-  window.top.location = name;
+  try {
+    var match = decodeURIComponent(window.location.hash).match(/^#!(.+)/);
+    var name = match ? match[1] : '<%= url_for_main %>';
+    var url = new URL(name, window.location.href);
+    if (url.origin === window.location.origin) {
+      window.top.location = url;
+    } else {
+      window.top.location = '<%= url_for_main %>';
+    }
+  } catch (e) {
+    window.top.location = '<%= url_for_main %>';
+  }
 </script>
 <noscript>
   <h1>Oops!</h1>
   <h2>YARD requires JavaScript!</h2>
 </noscript>
+
 </html>

2. Debian's suggestion (This vulnerability was found in their infrastructure, and they suggested this fix)

--- a/templates/default/fulldoc/html/frames.erb
+++ b/templates/default/fulldoc/html/frames.erb
@@ -7,7 +7,7 @@
 <script type="text/javascript">
   var match = unescape(window.location.hash).match(/^#!(.+)/);
   var name = match ? match[1] : '<%= url_for_main %>';
-  name = name.replace(/^(\w+):\/\//, '').replace(/^\/\//, '');
+  name = name.replace(/^(\w+):/, '').replace(/^\/\//, '');
   window.top.location = name;
 </script>
 <noscript>

3. The removal of the file, as it is not referenced anywhere, and serves no purpose.

Please let me know if you have any comments you'd like me to pass along.

Yesterday I have deleted the mediawiki-selenium documentation since the repository has long been archived and abandoned by us.

I don't think we have much knowledge about yardcode and I am definitely not qualified in reviewing a fix for the upstream code. It is best to collaborate with upstream if they are available and end up with a release fixing it. We can then upgrade the gems / rebuild the documentation.

The alternative is to fork the repository on our infra, apply a patch then update the Gemfile from each three repositories to install yard from our git fork. But that sounds like a lot of effort.

For when yard is fixed, below are the instructions as to how to update each of the three affected repositories and get the documentations regenerated.

mediawiki/vagrant

Gemfile.lock is committed and will need to be regenerated and committed to update yard: `

operations/puppet

There is no Gemfile.lock, thus the next build should use the latest yard version available.

The documentation is generated by https://integration.wikimedia.org/ci/job/operations-puppet-doc/ which polls the repository on an hourly basis. The job can be manually triggered by anyone from release engineering or by member of the LDAP ciadmin group. Once building, the job would show the output of bundler install which shows the version of the gem being fetched and installed:

Fetching yard 0.9.34
Installing yard 0.9.34

mediawiki/ruby/api

There is no Gemfile.lock and the documentation is only build after a change has merged rather than polling. So one would have to reenqueue in CI the last change that got merged in the postmerge queue. That is currently 968967,1 and the doc can be regenerated using:

ssh contint.wikimedia.org
zuul enqueue --trigger gerrit --pipeline postmerge --project mediawiki/ruby/api --change 968967,1

The change will show up at https://integration.wikimedia.org/zuul/ and would have https://integration.wikimedia.org/ci/job/mediawiki-ruby-api-ruby2.5-bundle-yard-publish/ mediawiki-ruby-api-ruby2.5-bundle-yard-publish Jenkins job running.

Once the three documentations have been regenerated, the old files are still served from the ATS/Varnish cache. There is an one hour TTL expiry.

Sounds good, I’ll let you know when Yard is fixed. Currently, we are still working on a patch.

The file has been removed from the puppet doc tree, so at least that's not a problem anymore.

All it took was a patch to the Rakefile to remove frames.html

Two down, two to go!

The three CI jobs now remove the frames.html file from the generated doc tree. The jobs do output:

removed 'frames.html'

I have verified all three CI jobs are working and the frames.html are gone from the public:

https://doc.wikimedia.org/mediawiki-vagrant/frames.html
https://doc.wikimedia.org/rubygems/mediawiki-ruby-api/frames.html
https://doc.wikimedia.org/rubygems/mediawiki-selenium/frames.html (I have removed the whole document tree since the repo has been archived)
https://doc.wikimedia.org/puppet/frames.html (which was done by Joe by having the repo Rakefile to delete the file. That https://gerrit.wikimedia.org/r/c/operations/puppet/+/1007304 can be rolled back if needed).

Fixed upstream in v0.9.35 via commit https://github.com/lsegal/yard/commit/2069e2bf08293bda2fcc78f7d0698af6354054be and also now public via https://github.com/lsegal/yard/security/advisories/GHSA-8mq4-9jjh-9xrc

I failed to patch the vulnerability. It is still vulnerable, see https://github.com/lsegal/yard/pull/1537. I take full responsibility for this oversight.

In T358507#9587042, @RedYetiDev wrote:

I failed to patch the vulnerability. It is still vulnerable, see https://github.com/lsegal/yard/pull/1537. I take full responsibility for this oversight.

Ok, we're still patched against the issue for now (disabling frames.html). Hopefully a new release of yard that completely addresses the issue will be tagged soon.

I’ll let you know when a new release comes out. I’ll see what I can do to resolve this, but my advice might not be wanted after I failed to patch the vulnerability the first-time around. (Sorry again)

Fixed in v0.9.36

Also fixed in Debian via DSA 5635: https://lists.debian.org/debian-security-announce/2024/msg00041.html

Hi everyone, seeing as this is now resolved, can we go ahead and mark it as Resolved? Or is there more that needs to be done

We've deployed a workaround via Puppet, we'll probably mark the task as resolved when the CI infrastructure has been upgraded to 0.9.36.

In T358507#9609721, @RedYetiDev wrote:

Hi everyone, seeing as this is now resolved, can we go ahead and mark it as Resolved? Or is there more that needs to be done

The full resolution needs yard to be upgraded in the three active repositories and the CI workaround (rm frames.html) to be removed.

Ah, okay, makes sense

Change 1010519 had a related patch set uploaded (by Hashar; author: Hashar):

[mediawiki/vagrant@master] Update yard to 0.9.36

https://gerrit.wikimedia.org/r/1010519

Change 1010223 had a related patch set uploaded (by Hashar; author: Hashar):

[integration/config@master] Revert "jjb: remove useless yard generated file"

https://gerrit.wikimedia.org/r/1010223

I have made the task public since we worked around it and the fix got released Upstream with v0.9.36.

For the three repositories we have:

mediawiki/ruby/api does not have a Gemfile.lock and an installation fetches the latest yard:

Fetching yard 0.9.36
Installing yard 0.9.36

operations/puppet does not have a Gemfile.lock and I have confirmed recent builds of https://integration.wikimedia.org/ci/job/operations-puppet-doc/ pick up yard 0.9.36:

Fetching yard 0.9.36
Installing yard 0.9.36

The last repository, mediawiki/vagrant, does have a Gemfile.lock and https://gerrit.wikimedia.org/r/c/mediawiki/vagrant/+/1010519 should get us yard 0.9.36. Once merged, the output of the Jenkins job would need to be checked to verify yard 0.9.36 is fetched.

Once merged, we can remove the frames.html removal from the CI jobs which is https://gerrit.wikimedia.org/r/c/integration/config/+/1010223

TLDR https://gerrit.wikimedia.org/r/c/mediawiki/vagrant/+/1010519 needs review/+2 ;)

Change 1010519 merged by jenkins-bot:

[mediawiki/vagrant@master] Update yard to 0.9.36

https://gerrit.wikimedia.org/r/1010519

Thanks, @hashar. Looks like all we're waiting on now is a +2/deploy for https://gerrit.wikimedia.org/r/q/Id099f2602c333bf5843fa66776662d7bbb9fd923 and then this task can be resolved?

Change 1010223 merged by jenkins-bot:

[integration/config@master] Revert "jjb: remove useless yard generated file"

https://gerrit.wikimedia.org/r/1010223

Change 1010570 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/puppet@production] Revert "Rakefile: remove useless files from generated docs"

https://gerrit.wikimedia.org/r/1010570

And I have caught another one which is that operations/puppet had another patch to remove the frames.html file: https://gerrit.wikimedia.org/r/1010570

Change 1010570 merged by Giuseppe Lavagetto:

[operations/puppet@production] Revert "Rakefile: remove useless files from generated docs"

https://gerrit.wikimedia.org/r/1010570

Thanks, all. @RedYetiDev - would you like to be added to our security hall-of-fame?

Sure! Thanks for the offer!

Change 1012651 had a related patch set uploaded (by SBassett; author: SBassett):

[wikimedia/security/landing-page@master] Add Aviv Keller to WMF Security Hall of Fame (data file)

https://gerrit.wikimedia.org/r/1012651

Change 1012651 merged by jenkins-bot:

[wikimedia/security/landing-page@master] Add Aviv Keller to WMF Security Hall of Fame (data file)

https://gerrit.wikimedia.org/r/1012651

Change 1012657 had a related patch set uploaded (by SBassett; author: SBassett):

[wikimedia/security/landing-page@master] Add Aviv Keller to WMF Security Hall of Fame (build step)

https://gerrit.wikimedia.org/r/1012657

Change 1012657 merged by jenkins-bot:

[wikimedia/security/landing-page@master] Add Aviv Keller to WMF Security Hall of Fame (build step)

https://gerrit.wikimedia.org/r/1012657

Thanks for the recognition!