Page MenuHomePhabricator

Cross-domain policy regexp is too narrow
Closed, ResolvedPublic

Description

Our regexp in OutputHandler, ApiFormatJson and ApiFormatPhp ('/\<\s*cross-domain-policy\s*\>/i') counts on this tag to never have any attributes. This is not the case, e.g. https://twitter.com/crossdomain.xml has <cross-domain-policy xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="http://www.adobe.com/xml/schemas/PolicyFile.xsd">. In any case, we must not assume that the spec will never change or that there will never be non-compliant plugin implementations.

Event Timeline

MaxSem raised the priority of this task from to Needs Triage.
MaxSem updated the task description. (Show Details)
MaxSem changed the visibility from "Public (No Login Required)" to "Custom Policy".
MaxSem changed the edit policy from "All Users" to "Custom Policy".
MaxSem changed Security from None to Software security bug.
MaxSem subscribed.

Alternatively, just nuke teh whole thing? Unless someone has an insane rewrite that allows to actually output this XML at /crossdomain.xml, this technique shouldn't be needed anymore.

I believe it's still needed, since any site can helpfully point to a custom url for crossdomain.xml on a site. I believe /crossdomain.xml will override the custom policy, but I haven't yet deployed a /crossdomain.xml yet (T75574: Host crossdomain.xml master policy file).

Anomie subscribed.

I think we should be able to get away with just changing the regex, along the lines of '/\<\s*cross-domain-policy(?=\s|>)/i', and the replacements similarly. This would be a slight change in that it would turn <cross-domain-policy> into \u003Ccross-domain-policy> instead of \u003Ccross-domain-policy\u003E for ApiFormatJson, but I don't think that will matter for the purpose.

This patch looks fine to me, although I wonder if it would be simpler to do /\<\s*cross-domain-policy/i instead

Well, that would catch things like <cross-domain-policy-foobar that aren't the problematic tag.

Updated for short array format in the tests.

dpatrick claimed this task.
dpatrick subscribed.

Patch deployed (https://wikitech.wikimedia.org/wiki/Server_Admin_Log):

22:47 dapatrick: Deployed patch for T123653 to wmf18 and wmf19

demon changed the visibility from "Custom Policy" to "Public (No Login Required)".May 20 2016, 5:24 PM
demon changed the edit policy from "Custom Policy" to "All Users".
demon changed Security from Software security bug to None.