Page MenuHomePhabricator

CVE-2025-53501: Scribunto title.getContent() doesn't respect $wgNonincludableNamespaces
Closed, ResolvedPublicSecurity

Description

Lua function title.getContent() of Scribunto can be used to bypass the read-restrictions even when $wgNonincludableNamespaces is set, which is a security issue.

Event Timeline

To further clarify, this is especially a problem for pages protected by Extension:Lockdown. Security issue because the stated purpose of this setting is "[a]mong other things, this may be useful to enforce read-restrictions that may otherwise be bypassed by using the template mechanism."

Reproduction in Lua debug console:

=mw.title.new('Protected_namespace:Page'):getContent()
Dianliang233 changed Author Affiliation from WMF Technology to N/A.Jun 20 2025, 2:52 PM
Dianliang233 added a subscriber: Lakejason0.

Another concern is that it seems like that even logged-out users can use the debug console (if * has read permission). Just provide the \+ token as the csrf and this API will happily execute the question. We might also want to check the edit permission to modules when using the Console API? But upon checking the debug console's code this seems intended...

In includes/Engines/LuaCommon/TitleLibrary.php, there is a method getContentInternal:

// ...
	/**
	 * Utility to get a Content object from a title
	 *
	 * The title is counted as a transclusion.
	 *
	 * @param string $text Title text
	 * @return Content|null The Content object of the title, null if missing
	 */
	private function getContentInternal( $text ) {
		$title = Title::newFromText( $text );
		if ( !$title || !$title->canExist() ) {
			return null;
		}
	// ...
	}
// ...

Is it sufficient to just add another call under "if the title exists", like so?

// ...
	/**
	 * Utility to get a Content object from a title
	 *
	 * The title is counted as a transclusion.
	 *
	 * @param string $text Title text
	 * @return Content|null The Content object of the title, null if missing
	 */
	private function getContentInternal( $text ) {
		$title = Title::newFromText( $text );
		if ( !$title || !$title->canExist() ) {
			return null;
		}

		if ( MediaWikiServices::getInstance()->getNamespaceInfo()->isNonincludable( $title->getNamespace() ) ) {
			return null;
		}
	// ...
	}
// ...

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Scribunto/+/1162401

@Lakejason0, Thank you for working on this issue. In the future can you put patches up privately as outlined in developing security patches. It would be ideal to get this patch merged today.

在T397524#10939279中,@Mstyles写道:

@Lakejason0, Thank you for working on this issue. In the future can you put patches up privately as outlined in developing security patches. It would be ideal to get this patch merged today.

Thank you for telling me there is a page (didn't found it and because another secu issue related also just accidentally are on Gerrit. so...), I'll follow it in other tickets.

Change #1162401 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@master] Make Scribunto title.getContent() respect $wgNonincludableNamespaces

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

We should... probably consolidate some of these.

Ugh, yea... I wouldn't know wherre to start...

@apaskulin told me the tech docs team can help, just file a ticket and tag #Tech-Docs-Team.

Change #1164531 had a related patch set uploaded (by XtexChooser; author: Lakejason0):

[mediawiki/extensions/Scribunto@REL1_43] Make Scribunto title.getContent() respect $wgNonincludableNamespaces

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

Change #1164532 had a related patch set uploaded (by XtexChooser; author: Lakejason0):

[mediawiki/extensions/Scribunto@REL1_44] Make Scribunto title.getContent() respect $wgNonincludableNamespaces

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

Change #1164533 had a related patch set uploaded (by XtexChooser; author: Lakejason0):

[mediawiki/extensions/Scribunto@REL1_42] Make Scribunto title.getContent() respect $wgNonincludableNamespaces

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

Change #1164534 had a related patch set uploaded (by XtexChooser; author: Lakejason0):

[mediawiki/extensions/Scribunto@REL1_39] Make Scribunto title.getContent() respect $wgNonincludableNamespaces

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

Change #1164541 had a related patch set uploaded (by XtexChooser; author: XtexChooser):

[mediawiki/extensions/Scribunto@master] Add tests for NonincludableNamespaces restriction

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

Change #1164534 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@REL1_39] Make Scribunto title.getContent() respect $wgNonincludableNamespaces

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

Change #1164533 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@REL1_42] Make Scribunto title.getContent() respect $wgNonincludableNamespaces

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

Change #1164532 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@REL1_44] Make Scribunto title.getContent() respect $wgNonincludableNamespaces

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

Change #1164531 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@REL1_43] Make Scribunto title.getContent() respect $wgNonincludableNamespaces

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

Jly renamed this task from Scribunto title.getContent() doesn't respect $wgNonincludableNamespaces to CVE-2025-53501: Scribunto title.getContent() doesn't respect $wgNonincludableNamespaces.Jun 30 2025, 7:27 PM

Change #1164541 merged by jenkins-bot:

[mediawiki/extensions/Scribunto@master] Add tests for NonincludableNamespaces restriction

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

Jly claimed this task.

FYI I split the public docs aspect into T398648.

Jly changed the visibility from "Custom Policy" to "Public (No Login Required)".
Jly changed the edit policy from "Custom Policy" to "All Users".
Jly changed Risk Rating from N/A to Low.

This fix seems incorrect to me, as demonstrated by T399081. Wikifunctions is an example where the Z-function pages, for example [[Z42]] "should not be transcluded" but that doesn't mean that their contents are somehow secret or shouldn't be accessed by Scribunto; in fact as T399081 certain templates relied on that. To me it seems more that Extension:Lockdown was at fault, in assuming that simply setting "can not be transcluded" would make the pages completely inaccessible. If we want to have that functionality in core, it seems like a different name is needed.

That seems rather bad...

This fix seems incorrect to me, as demonstrated by T399081. Wikifunctions is an example where the Z-function pages, for example [[Z42]] "should not be transcluded" but that doesn't mean that their contents are somehow secret or shouldn't be accessed by Scribunto; in fact as T399081 certain templates relied on that. To me it seems more that Extension:Lockdown was at fault, in assuming that simply setting "can not be transcluded" would make the pages completely inaccessible. If we want to have that functionality in core, it seems like a different name is needed.

Right... I think the logic we would want is: page A can use information from B if read access to B does not require more privileges that access to page A. Transclusion should apply this check, and so should scribunto. PermissionManager has this information in principle, though I'm not sure there's a nice and easy way to get it out, nor am I sure whether it would work with existing hooks.

In any case, keep in mind that Lockdown is a dirty hack to approximate something that MediaWiki isn't designed to do, namely prevent read access to certain pages. It will never be 100% secure.

Setting $wgNonincludableNamespaces to WikiLambda ZObject pages on Wikifunctions seems pretty weird to me, as we don't do it for Scribunto Lua module pages.

I guess what really wanted/needed is something $wgNontranscludableContentModels.

This comment was removed by daniel.

This fix seems incorrect to me, as demonstrated by T399081

Incorrect enough that we should revert the patch, publish a correction and note the issue on ext:Lockdown's doc page?

To me it seems more that Extension:Lockdown was at fault, in assuming that simply setting "can not be transcluded" would make the pages completely inaccessible. If we want to have that functionality in core, it seems like a different name is needed.

I'm not sure how complex adding core functionality like this would be, but I assume this would at least partially revert or work around the security patch in question here.

在T397524#10992178中,@sbassett写道:

Incorrect enough that we should revert the patch, publish a correction and note the issue on ext:Lockdown's doc page?

Even if we do this, I don't really think there is a good workaround for this (since debug console of Lua can be used by anon as long as read permission is there, and basically if ext:Lockdown is used it would, of course, be used on public wikis) so that would just make it "oh just cherry pick this" (which sounds kinda suspicious and will be easily outdated) or "no you will have to disable the debug console for everyone; or write a hook to meet your need (and write no info on how to write a hook because of course MediaWiki /j)" or sth.

I kinda do think we are kinda acting like "of course would know that Ext:Lockdown won't lock down everything" but "if you mentioned one we would still find a way to fix it", so hopefully this could still be somehow addressed...

And do note that I make it respect that setting due to the comment of getContentInternal says "The title is counted as a transclusion" so I think my fix isn't like "too wrong" (though I did have doubt that it could be wrong since "maybe that's too rough and might make sth that should be accessible no longer accessible") anyway.

There are other issues which make Lockdown effectively useless in public wikis. See {T299359}

在T397524#10996495中,@Bugreporter写道:

There are other issues which make Lockdown effectively useless in public wikis. See {T299359}

Too bad that I couldn't see anything there...

And do note that I make it respect that setting due to the comment of getContentInternal says "The title is counted as a transclusion" so I think my fix isn't like "too wrong" (though I did have doubt that it could be wrong since "maybe that's too rough and might make sth that should be accessible no longer accessible") anyway.

Indeed. I think Wikifunctions shouldn't use NonincludeableNamespces for this, they should instead make getWikitextFortransclusion() return false. See T399081#10994997.

they should instead make getWikitextFortransclusion() return false.

We should never assume non-Wikitext (other than plaintext) is includable, so we can even make it false by default. For example currently we can transclude a MassMessage list, but this is ugly - raw JSON is shown in Wikitext page. So if we need to make something non-wikitext includable, we need to explicitly define mechanism to convert them to wikitext. Note Lua getContent() is not affected and can still read raw content.

Note we currently can transclude a .js to another page and https://en.wikipedia.org/wiki/Wikipedia:Protection_policy#User_pages suggests this way to protect user page from vandalism. This will nevertheless no longer be usable once T76204: Enforce JavaScript syntax check when editing user/site script pages is resolved.

People can still use CSS pages for this if we have JS syntax checking.