Page MenuHomePhabricator
Create Task
Maniphest T278365

CVE-2023-29138: Using checkuser api module with bad user name can still break Special:CheckUserLog even after security fixes
Closed, ResolvedPublicSecurity
Actions

Description

With T275669: Checkuser stores users to cu_log with trailing spaces, allowing all CUs to turn off Special:CheckuserLog at will (CVE-2021-31553) the problem with bad user input was fixed with a trim(), but using an trailing underscore instead of a trailing space can still break the special page, because the underscore is also invalid at the end or begin on Special:Contributions/<username>_

This should use proper title normalization before storing to the database (Special:CheckUser does that, api module checkuser not).

This can be done also by using the api param type 'user' with the user types 'name', 'ip', 'cidr'

Stack trace is the same as in the linked bug.

Details

SubjectRepoBranchLines +/-
SECURITY: Make the target safe before running a check in the APImediawiki/extensions/CheckUserREL1_35+3 -0
SECURITY: Make the target safe before running a check in the APImediawiki/extensions/CheckUserREL1_38+4 -0
SECURITY: Make the target safe before running a check in the APImediawiki/extensions/CheckUserREL1_39+4 -0
SECURITY: Make the target safe before running a check in the APImediawiki/extensions/CheckUserREL1_35+336 -0
SECURITY: Make the target safe before running a check in the APImediawiki/extensions/CheckUserREL1_40+4 -0
Customize query in gerrit

Related Objects

Event Timeline

Umherirrender created this task.Mar 24 2021, 6:12 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 24 2021, 6:12 PM
Reedy added a project: CheckUser.Mar 24 2021, 8:49 PM
sbassett edited projects, added SecTeam-Processed; removed Security-Team.Apr 5 2021, 3:24 PM
sbassett added a subscriber: Urbanecm.
Zabe added subscribers: Dreamy_Jazz, Zabe.Jan 8 2023, 4:50 PM
Dreamy_Jazz triaged this task as Medium priority.Jan 15 2023, 11:39 AM
Dreamy_Jazz raised the priority of this task from Medium to High.Jan 29 2023, 10:40 PM

It still breaks Special:CheckUserLog using the API.

Dreamy_Jazz claimed this task.Jan 29 2023, 10:59 PM
Dreamy_Jazz added a project: Patch-For-Review.

Proposed patch:

T278365.patch1 KBDownload

Dreamy_Jazz added a project: Security-Team.Jan 29 2023, 11:00 PM
Dreamy_Jazz added a comment.Jan 29 2023, 11:03 PM

Re-adding the security team as once this is +2'd it will need a deploy.

Zabe added a comment.Jan 30 2023, 8:22 AM
In T278365#8567821, @Dreamy_Jazz wrote:

Proposed patch:

T278365.patch1 KBDownload

deployed: https://sal.toolforge.org/log/8bjCAYYBa_6PSCT9YGb_

Mstyles moved this task from Incoming to Watching on the Security-Team board.Jan 30 2023, 5:13 PM
Mstyles subscribed.

Thanks you @Zabe for deploying this patch!

Mstyles mentioned this in T325849: Write and send supplementary release announcement for extensions and skins with security patches (1.35.10/1.38.6/1.39.3).Jan 30 2023, 5:16 PM
sbassett removed a project: Patch-For-Review.Jan 30 2023, 5:28 PM
dduvall subscribed.Feb 14 2023, 6:12 PM

I've refactored the patch following application failure during scap stage-train.

01-T278365.patch1 KBDownload

https://sal.toolforge.org/log/BUceUYYB0IdT-khD-sOb

dduvall added a comment.Feb 14 2023, 6:13 PM

I should clarify. I did not refactor the security patch implementation, only the context so it would apply cleanly.

sbassett subscribed.Feb 14 2023, 6:17 PM
In T278365#8615498, @dduvall wrote:

I should clarify. I did not refactor the security patch implementation, only the context so it would apply cleanly.

Ok, thanks, @dduvall!

Dreamy_Jazz added a comment.Feb 14 2023, 6:40 PM

Thanks for the refactoring and for the info.

mmartorana renamed this task from Using checkuser api module with bad user name can still break Special:CheckUserLog even after security fixes to CVE-2023-29138: Using checkuser api module with bad user name can still break Special:CheckUserLog even after security fixes.Apr 3 2023, 3:20 PM
mmartorana closed this task as Resolved.Apr 3 2023, 3:22 PM
Dreamy_Jazz added a comment.EditedApr 4 2023, 4:42 PM

I've backported the fix to 1.40, 1.39, 1.38 and 1.35 (based on the Version lifecycle).

One outstanding question is whether the maintenance script to trim the target text is needed to be run again as the fix for the API was only implemented now? Also whether this should be run automatically when running update.php?

mmartorana changed the visibility from "Custom Policy" to "Public (No Login Required)".Apr 4 2023, 7:05 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits