Page MenuHomePhabricator
Create Task
Maniphest T356971

Rename help key to help-raw in HTMLForm and deprecate old key name
Open, LowPublicSecurity
Actions

Description

The HTMLForm library doesn't escape the input of the help key by default. Is this intended behavior?

For example, the following form descriptor using an interface message through wfMessage()->text() is an XSS vector.

$formDescriptor= [
  'simpletextfield' => [
     'label' => 'Simple Text Field',
     'class' => 'HTMLTextField',
     'help' => wfMessage( 'myextension-mymessage' )->text(), // not escaped by HTMLForm, XSS!
   ]
]

We should rename the key to be help-raw, so users know it is a raw key. We should mark the old key name deprecated but keep it around for a transition period.

Afterwards we should adjust phan-taint-check to make sure it recognizes the new key name.

If you are fixing this, you probably need to edit (the comment in) includes/htmlform/HTMLForm.php and includes/htmlform/HTMLFormField.php as well as mentioning the deprecation in RELEASE-NOTES

Details

Risk Rating
Low
Author Affiliation
Wikimedia Communities

Related Objects

Event Timeline

AllUsernamesArePicked created this task.Feb 8 2024, 10:09 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 8 2024, 10:09 AM
AllUsernamesArePicked added a subscriber: Universal_Omega.Feb 8 2024, 10:10 AM
Reedy added projects: Vuln-XSS, MediaWiki-HTMLForm.Feb 8 2024, 3:15 PM
sbassett subscribed.Feb 8 2024, 5:01 PM

I believe this is intentional, as some help messages can contain small amounts of harmless html for formatting purposes. This is a pattern we'd likely try to correct though, if found in actual code. And we do that in various ways (see also T347742 et al). Otherwise it's generally incumbent upon an engineer or developer to have a good understanding the MediaWiki Messages API and its output modes.

Universal_Omega added a comment.Feb 8 2024, 5:52 PM
In T356971#9525868, @sbassett wrote:

I believe this is intentional, as some help messages can contain small amounts of harmless html for formatting purposes. This is a pattern we'd likely try to correct though, if found in actual code. And we do that in various ways (see also T347742 et al). Otherwise it's generally incumbent upon an engineer or developer to have a good understanding the MediaWiki Messages API and its output modes.

Is there a reason we can't make it use something like 'raw' to make it possible to not escape, I was wondering?

sbassett added a comment.Feb 8 2024, 6:03 PM
In T356971#9526226, @Universal_Omega wrote:

Is there a reason we can't make it use something like 'raw' to make it possible to not escape, I was wondering?

There isn't any technical reason, no. But it might break a bunch of existing help messages and irritate other developers who want this kind of flexibility. If we were going to go this route, we should just make this bug public and put up a gerrit change set for feedback and review.

Bawolff subscribed.Feb 8 2024, 6:48 PM

I've always hated that the key is named help instead of help-raw or help-html. Its super confusing. I'd support making a new name for the key and deprecating the old name.

AllUsernamesArePicked added a comment.Feb 8 2024, 6:51 PM
In T356971#9526494, @Bawolff wrote:

I've always hated that the key is named help instead of help-raw or help-html. Its super confusing. I'd support making a new name for the key and deprecating the old name.

+1. If nothing it would at least make it consistent with the naming scheme used in for example label/label-raw.

sbassett added a comment.Feb 9 2024, 4:48 PM
In T356971#9526517, @AllUsernamesArePicked wrote:
In T356971#9526494, @Bawolff wrote:

I've always hated that the key is named help instead of help-raw or help-html. Its super confusing. I'd support making a new name for the key and deprecating the old name.

+1. If nothing it would at least make it consistent with the naming scheme used in for example label/label-raw.

patch-welcome. If someone is willing to get started on that, we can definitely make this task public.

AllUsernamesArePicked added a comment.Feb 9 2024, 6:57 PM

I'll give it a try. I need some experience with gerrit, and this looks like a good chance.

Bawolff added a comment.Feb 9 2024, 7:28 PM

If you aren't familiar, be sure to read through https://www.mediawiki.org/wiki/Stable_interface_policy#Deprecation_process

sbassett triaged this task as Low priority.Feb 12 2024, 5:24 PM
sbassett changed Author Affiliation from N/A to Wikimedia Communities.
sbassett edited projects, added SecTeam-Processed; removed Security-Team.
sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".
sbassett changed the edit policy from "Custom Policy" to "All Users".
sbassett changed Risk Rating from N/A to Low.
AllUsernamesArePicked added a comment.Feb 17 2024, 10:32 AM

Sorry, won't be able to work on this after all.

Bawolff added a comment.Feb 17 2024, 9:02 PM
In T356971#9552938, @AllUsernamesArePicked wrote:

Sorry, won't be able to work on this after all.

No worries. Patches are always welcome but never an obligation. Raising the issue is in and of itself a contribution to MediaWiki :)

Bawolff renamed this task from HTMLForm doesn't escape the help key by default to Rename help key to help-raw in HTMLForm and deprecate old key name.Feb 17 2024, 9:07 PM
Bawolff added a project: good first task.
Bawolff updated the task description. (Show Details)
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL