Not sure about the page_id non-object notice, but this does not appear to be a legitimate security issue. The XSS within the example in the task description doesn't render, and shouldn't, given the relevant code is validating limit to be an int within a certain range. The rest of the default/form options also appear to be well-validated. From the errors I'm seeing in logstash, it looks like someone ran a bunch of basic XSS, SQLi, etc. payloads against the limit request parameter which did not succeed in triggering any of the intended vulnerabilities but did induce the page_id non-object notice.

I guess it is of marginal interest that these all seem to be against the same enwiki user's talk page, but that could mean anything, and the only elevated privilege they have is patroller on enwiki. One other item of note - some of the payloads seem to reference the bxss.me domain, which would imply commercial scanners from Acunetix, which are not inexpensive.

These are still happening from time to time, generating log noise.

Not that this needs to be a security patch (it can go through gerrit) but this should fix the immediate issue of better handling the non-object error. Tested fine locally against master (89259d5a61):

In T289351#7306214, @sbassett wrote:

Not that this needs to be a security patch (it can go through gerrit) but this should fix the immediate issue of better handling the non-object error. Tested fine locally against master (89259d5a61):

The patch seems to hide the underlying issue, rather than fix it. Basically, $rows is a list of database rows -- except that apparently, one of the entries in $rows is actually not. It looks like $rows is put together by iterating over ResultSets, it should not be possible for them to return things that are note database rows.

I agree that this doesn't seem to be exploitable, but it's not clear to me what is actually happening. In particular, it's not clear whether this request manages to do something odd in a database query.

You said you tested it locally - were you able to reproduce the error locally as well? Can you describe the steps you took?

I set the policy to public, since this is not a security issue.

In T289351#7306626, @daniel wrote:

The patch seems to hide the underlying issue, rather than fix it. Basically, $rows is a list of database rows -- except that apparently, one of the entries in $rows is actually not. It looks like $rows is put together by iterating over ResultSets, it should not be possible for them to return things that are note database rows.

I don't think they are, at least not intentionally. Looking at this a bit more, the page_id non-object error seems to be related to the quirk of php casting strings to an integer value of 0. So, in the example within the task description, when limit is set to a string (e.g. an XSS payload), the conditionals comparing $numRows to $limit in the relevant block within includes/specials/SpecialWhatLinksHere.php allow code to execute that shouldn't. Yes, my patch papers over this and doesn't necessarily fix the underlying issue. I'm not entirely sure what the best solution would be though - certainly we don't just want to throw fatals here for non-integer values of limit since that wouldn't address the logspam issue.

You said you tested it locally - were you able to reproduce the error locally as well? Can you describe the steps you took?

Yes, I tested locally within mediawiki-docker by both hard-coding some data injections and by browsing to links like the one within the task description, with different fuzzed values of limit.

It is also possible to trigger an error with limit 0 - https://en.wikipedia.org/wiki/Special:WhatLinksHere/Main_Page?limit=0
All the strings are treated as 0 and therefor the same.

Maybe a upper limit of 1 is better for a limit parameter

I considered just passing 1 for the minimum limit when it calls validateIntBounds(), here and in the other special pages that call that function. But the other special pages apparently work when limit=0 is passed, and this one can also work with a trivial patch. Showing no results for limit=0 seems to best follow the user's intentions, and there might be valid reasons for doing that, for example to view the headers and footers on a special page.

Change 715346 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] Fix notice for Special:WhatLinksHere?limit=0

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

Change 715548 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/core@master] Make Special:WhatLinksHere ignore limit=

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

Change 715346 merged by jenkins-bot:

[mediawiki/core@master] Fix notice for Special:WhatLinksHere?limit=0

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

Change 774471 had a related patch set uploaded (by Reedy; author: Tim Starling):

[mediawiki/core@REL1_36] Fix notice for Special:WhatLinksHere?limit=0

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

Change 774471 abandoned by Reedy:

[mediawiki/core@REL1_36] Fix notice for Special:WhatLinksHere?limit=0

Reason:

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

Change 715548 merged by jenkins-bot:

[mediawiki/core@master] Fix Special:WhatLinksHere behavior on limit= vs. limit=0

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