Change 549953 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] [WIP] preferences: Prevent "nested" substitution in signature

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

Change 549953 abandoned by Bartosz Dziewoński:
[WIP] preferences: Prevent "nested" substitution in signature

Reason:
It'll be easier for me to work on this and related changes in one patch, see: I07c575c2d9d2afe7a89c4847d16ac044417297bf

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

Change 569062 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] preferences: Signature validation (lint errors, user links, nested subst)

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

Per the description - This isn't really a security bug - I'm going to un-sub Security-Team for now.

Once we get community input as mentioned in T244519 we can work on this task.

Hello, as I read, this change targets "nested" subst's. Does this code fall into that? It changes User talk:Jack who built the house link to User talk:Jack who built the house#top if I post on my talk page.

— [[User:Jack who built the house|Jack who built the house]] {{subst:#ifeq: {{subst:FULLPAGENAME}} | {{subst:ns:3}}:Jack who built the house | ([[User talk:Jack who built the house#top|talk]]) | ([[User talk:Jack who built the house|talk]]) }}

No, it only disallows syntax that produces another {{subst:...}} or ~~~... after the substitution (which would be then substituted again when the next user edits the page).

I could not think of a better way to describe it, "nested" subst is not a very precise description. I'd love to update the documentation page with a better one :)

I experimented with inserting my local time in my sig. At one stage I had nested {{subst:#time … {{subst:#expr … {{subst:#time …}}}}}} (subst one, subst 'em all), which would be banned under this proposal.

Fortunately I discovered that #time can take parameters like "+10 hours" and do it in one call. But there are potentially other legitimate reasons for multi subst'ing in sigs.

If the actual test is more subtle per @matmarex's post T230652#5966006, then there shouldn't be much problem. Perhaps the title could say "chained subst" or "evil subst", rather than "nested"? In the final communication, you could use a phrase like "certain pathological uses of subst:" or "abuses of subst:" to make it clear that the exclusion will be tightly focussed.

Task description updates
Adding the following sections to the task description; please let me know if anything looks unexpected.

• Dependencies
• Requirements
• Done

Blocked
This task will be unblocked once the following tasks are resolved:

• T254613
• T254614

So… this is a duplicate of my ancient T14495, right? :-)

@Mormegil It's a bit different. Your task is about disallowing signatures that produce {{blah}} after substitution. This task is about disallowing signatures that produce ~~~~ or {{subst:blah}} (etc.) after substitution, which would then be substituted again after another person edits the page.

Change 569062 merged by jenkins-bot:
[mediawiki/core@master] preferences: Signature validation (lint errors, user links, nested subst)

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

Like in T237700#6286867, @matmarex can you:

1. REVIEW the "Testing details" section of the task description to ensure it is accurate and exhaustive?

cc @Ryasmeen

LGTM

@matmarex:

I followed the testing details and for signature entered this exactly: ~~{{subst:1x{{subst:1x|{{subst:!}}}}}}~~

And it does show the appropriate message as expected.

Is that all I need to check for this?

I tried couple of other things, although not sure if those are part of validation checks for this. For example: I tried adding tildes as part of my username with the checkbox being both checked and unchecked. For both cases, it discards the tildes after saving the changes, however it does not show any error messages.

Just thought to add this observation here.

In T230652#6291947, @Ryasmeen wrote:

@matmarex:

I followed the testing details and for signature entered this exactly: ~~{{subst:1x{{subst:1x|{{subst:!}}}}}}~~

And it does show the appropriate message as expected.

Is that all I need to check for this?

I think that's good enough…

If you want to dig at it, here are some more test cases (as used for unit tests), which might provide some inspiration: https://github.com/wikimedia/mediawiki/blob/master/tests/phpunit/includes/preferences/SignatureValidatorTest.php#L69-L72

We could also double-check that just using a template in the signature doesn't cause this error.

I tried couple of other things, although not sure if those are part of validation checks for this. For example: I tried adding tildes as part of my username with the checkbox being both checked and unchecked. For both cases, it discards the tildes after saving the changes, however it does not show any error messages.

Just thought to add this observation here.

That's pre-existing code, which indeed tries to automatically fix this problem by removing the tildes, but it only handles the simple cases. I'm not sure if we should try to remove that now, I think it's fine.