Page MenuHomePhabricator

Do not show oversighted edit summaries in CheckUser API (CVE-2019-18611)
Closed, ResolvedPublic

Description

Per, @Umherirrender's comment at T207094#5553504: we need to verify whether or not the API is leaking the same information as the issue within the parent task and, if it is, create and deploy a security patch.

Event Timeline

Basically duplicated my code from the previous patch to the API module, thanks for spotting this. Not sure how to incorporate @Umherirrender's edits made in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/541606, any suggestions?

@Urbanecm -

  1. + 1 to the patch above (T234862#5558854) as-is. It mostly mirrors the patch for T207094, which should be fine.
  2. To incorporate @Umherirrender's changes from r541606, I think we could do something like this in includes/api/ApiQueryCheckUser.php. Once r541606 is merged, we could maybe make loadRevisionRecords() static, add the appropriate use statements and do:
// around line 112 w/ T234862#5558854 applied
$revisionRecord = SpecialCheckUser::loadRevisionRecords( $result )
// around lines 126 to 149 w/ T234862#5558854 applied
$revisionRecord = $revisionRecords[$row->cuc_this_oldid] ?? false;
if ( $revisionRecord !== false &&
    $revisionRecord->audienceCan(
        RevisionRecord::DELETED_COMMENT, RevisionRecord::FOR_THIS_USER, $this->getUser()
    )
)

We might just want to get this security-deployed as-is for now (with the patch from T234862#5558854) and push the improvements through gerrit as is being done for the parent patch, as I believe this version of the code shouldn't have any performance issues.

@Urbanecm - did you want to deploy your patch from T234862#5558854? If not, I'm happy to deploy it on Monday (2019-10-21) during the security deployment window.

Patch was deployed today.

Thanks, I'll request a CVE soon. Also tracking in T234983.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Oct 28 2019, 9:24 PM

Change 546741 had a related patch set uploaded (by SBassett; owner: Urbanecm):
[mediawiki/extensions/CheckUser@master] SECURITY: Do not show oversighted edit summaries in CheckUser API

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

Change 546742 had a related patch set uploaded (by SBassett; owner: Urbanecm):
[mediawiki/extensions/CheckUser@REL1_34] SECURITY: Do not show oversighted edit summaries in CheckUser API

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

Change 546743 had a related patch set uploaded (by SBassett; owner: Urbanecm):
[mediawiki/extensions/CheckUser@REL1_33] SECURITY: Do not show oversighted edit summaries in CheckUser API

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

Change 546745 had a related patch set uploaded (by SBassett; owner: Urbanecm):
[mediawiki/extensions/CheckUser@REL1_32] SECURITY: Do not show oversighted edit summaries in CheckUser API

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

Change 546746 had a related patch set uploaded (by SBassett; owner: Urbanecm):
[mediawiki/extensions/CheckUser@REL1_31] SECURITY: Do not show oversighted edit summaries in CheckUser API

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

Change 546746 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@REL1_31] SECURITY: Do not show oversighted edit summaries in CheckUser API

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

Change 546745 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@REL1_32] SECURITY: Do not show oversighted edit summaries in CheckUser API

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

Change 546742 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@REL1_34] SECURITY: Do not show oversighted edit summaries in CheckUser API

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

Change 546743 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@REL1_33] SECURITY: Do not show oversighted edit summaries in CheckUser API

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

Change 546741 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] SECURITY: Do not show oversighted edit summaries in CheckUser API

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

Backports to supported release branches merged, CVE requested. I'll update this task and T234983 once I have the CVE ID.

sbassett renamed this task from Verify that the MediaWiki API is not leaking Oversighted edit summaries within CheckUser results to Verify that the MediaWiki API is not leaking Oversighted edit summaries within CheckUser results (CVE-2019-18611).Oct 29 2019, 3:48 PM
sbassett renamed this task from Verify that the MediaWiki API is not leaking Oversighted edit summaries within CheckUser results (CVE-2019-18611) to Do not show oversighted edit summaries in CheckUser API (CVE-2019-18611).Dec 19 2019, 6:10 PM