Page MenuHomePhabricator

No notification is sent when signature contains links to subpages in user namespace in addition to the required link to the user page itself
Closed, ResolvedPublic

Description

When a signature contains a link to a subpage in user namespace the name of the user should be extracted by only looking at the base page. Currently such links prevent notifications to be sent, even when a normal link is present, too. For example https://de.wikipedia.org/w/index.php?title=Benutzer_Diskussion%3ASchnark&diff=138275105&oldid=138274875 didn't create a notification.

Details

Related Gerrit Patches:
mediawiki/extensions/Echo : wmf/1.25wmf16Improve signature detection
mediawiki/extensions/Echo : masterImprove signature detection
mediawiki/extensions/Echo : wmf/1.25wmf15Improve signature detection

Event Timeline

Schnark created this task.Jan 29 2015, 9:29 AM
Schnark raised the priority of this task from to Needs Triage.
Schnark updated the task description. (Show Details)
Schnark added a project: Notifications.
Schnark added a subscriber: Schnark.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 29 2015, 9:29 AM
FriedhelmW set Security to None.Jan 29 2015, 9:45 AM
FriedhelmW added subscribers: Quiddity, FriedhelmW.
FriedhelmW triaged this task as Low priority.Jan 29 2015, 10:41 AM

This is not a frequent case.

Not frequent? For example 3 out of 8 users in this section link to a subpage in their signature.

They shouldn't.

They shouldn't.

Why? Such signatures have been common for years. Please note that they the link to the subpage is only additional, the signatures contain a link to the user page, too, but as I wrote the ping still fails.

Wondering if https://gerrit.wikimedia.org/r/#/c/173455/ (merged but not deployed yet, for T75426) will also influence this issue.

How should that influence this issue? The problem is in https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FEcho/2578c37d6c5eac7f128be35210407ee5fccef688/includes%2FDiscussionParser.php#L709, if a user links to to User:Name/subpage, the code detects "Name/subpage" as the name of the user, because it doesn't strip the subpage from the link. But then the signing user and the editing user are different, so the signature is not recognized as valid.

Mattflaschen-WMF renamed this task from No notification is sent when signature contains links to subpages in user namespace to No notification is sent when signature contains links to subpages in user namespace in addition to the required link to the user page itself.Feb 3 2015, 7:08 PM
gerritbot added a subscriber: gerritbot.

Change 187850 had a related patch set uploaded (by Matthias Mullie):
Improve signature detection

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

Patch-For-Review

Change 187850 merged by jenkins-bot:
Improve signature detection

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

Change 189548 had a related patch set uploaded (by EBernhardson):
Improve signature detection

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

Patch-For-Review

Change 189548 merged by jenkins-bot:
Improve signature detection

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

Change 189549 had a related patch set uploaded (by EBernhardson):
Improve signature detection

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

Patch-For-Review

Change 189549 merged by jenkins-bot:
Improve signature detection

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

EBernhardson closed this task as Resolved.Feb 9 2015, 8:28 PM
EBernhardson claimed this task.
EBernhardson added a subscriber: EBernhardson.

I've added this fix to todays SWAT deploy, it should be live to all production wiki's in 4 to 5 hours.

Schnark reopened this task as Open.Feb 10 2015, 9:03 AM

Is it live now on de.wikipedia? If so, the patch didn't fix the issue, I didn't get a notification for https://de.wikipedia.org/w/index.php?title=Benutzer_Diskussion%3ASchnark&diff=138679710&oldid=138602080

Although it was supposed to go out to both deployment branches, it seems it only made it to 1.25wmf16 (de.wiki is on 1.25wmf15 so didn't yet get this patch)
All remaining wikis will roll over to 1.25wmf16 tonight, so if you could test again tomorrow, that would be great :)

@Schnark: Any chance you could try again?

I wanted to try myself with a username similar to X" onclick="alert('XSS');" title="y, but wasn't able to register that one on dewiki - disallowed (which, should the ping still not work, may be one of the reasons why?)

... nevermind, the code is apparently not on de.wiki at the moment - we'll look to deploy it soon :)

@Schnark: Any chance you could try again?
I wanted to try myself with a username similar to X" onclick="alert('XSS');" title="y, but wasn't able to register that one on dewiki - disallowed (which, should the ping still not work, may be one of the reasons why?)

After I registered that name, the titleblacklist has been made stricter, but I think it's just one or two characters that the name is now too long, just leave the spaces out and change the x and y for some other letters and it should work. At least, I was able to register Benutzer:A'onclick='alert("XSS");'title='b after the change to the blacklist.

@Schnark: I give up. I was able to register a similar username (X"onclick="alert('X');"rel="z), but when I tried to ping that one a couple of minutes later, I got:
"Du kannst dich nicht anmelden, da dein Benutzerkonto global gesperrt ist."

Any chance you could test this again? The code should now be on de.wiki.

The comments are combining three different issues:

  1. Signatures with a link to a subpage in addition to the user page (e.g. "--[[Benutzer:Schnark]] [[Benutzer:Schnark/js|js]] 10:00, 10. Feb. 2015 (CET)")
  2. Signatures with extra parenthetical text
  3. Some kind of XSS (there is no report that Echo or Flow is actually vulnerable to XSS here yet, though).

This bug is only about #1, which is easily testable without #2 and #3.

Schnark closed this task as Resolved.Feb 13 2015, 8:48 AM

https://de.wikipedia.org/w/index.php?title=Benutzer_Diskussion%3ASchnark&diff=138781806&oldid=138681642 did work as expected, so this (and every other possible bug that could occur in this situation) really is fixed.