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.
Description
Details
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Restricted Task | |||||
Resolved | Urbanecm | T234862 Do not show oversighted edit summaries in CheckUser API (CVE-2019-18611) |
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?
- + 1 to the patch above (T234862#5558854) as-is. It mostly mirrors the patch for T207094, which should be fine.
- 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.
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
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
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
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
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
Change 546746 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@REL1_31] SECURITY: Do not show oversighted edit summaries in CheckUser API
Change 546745 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@REL1_32] SECURITY: Do not show oversighted edit summaries in CheckUser API
Change 546742 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@REL1_34] SECURITY: Do not show oversighted edit summaries in CheckUser API
Change 546743 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@REL1_33] SECURITY: Do not show oversighted edit summaries in CheckUser API
Change 546741 merged by jenkins-bot:
[mediawiki/extensions/CheckUser@master] SECURITY: Do not show oversighted edit summaries in CheckUser API
Backports to supported release branches merged, CVE requested. I'll update this task and T234983 once I have the CVE ID.