Page MenuHomePhabricator
Create Task
Maniphest T308861

CVE-2022-39191: OAuth debug log includes consumer secrets
Closed, ResolvedPublicSecurity
Actions

Description

On mwlog1002, /srv/mw-log/OAuth.log contains oauth_token_secret query parameters. This is unnecessary for debugging and creates a needless risk.

Details

Risk Rating
Medium
Author Affiliation
Wikimedia Communities
ProjectBranchLines +/-Subject
mediawiki/extensions/OAuthREL1_37+1 -1Fix cache var name
mediawiki/extensions/OAuthREL1_35+1 -1Fix cache var name
mediawiki/extensions/OAuthREL1_38+1 -1Fix cache var name
mediawiki/extensions/OAuthREL1_38+6 -1SECURITY: redact oauth_token_secret within log data
mediawiki/extensions/OAuthREL1_39+5 -0SECURITY: redact oauth_token_secret within log data
mediawiki/extensions/OAuthREL1_37+6 -1SECURITY: redact oauth_token_secret within log data
mediawiki/extensions/OAuthmaster+5 -0SECURITY: redact oauth_token_secret within log data
mediawiki/extensions/OAuthREL1_35+6 -1SECURITY: redact oauth_token_secret within log data
Customize query in gerrit

Related Objects

Event Timeline

awight created this task.May 20 2022, 2:14 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 20 2022, 2:14 PM
Reedy added projects: MediaWiki-extensions-OAuth, Vuln-Infoleak.May 21 2022, 8:28 AM
sbassett moved this task from Incoming to Watching on the Security-Team board.May 27 2022, 3:55 PM
sbassett added a project: SecTeam-Processed.
sbassett changed Author Affiliation from N/A to Wikimedia Communities.
sbassett changed Risk Rating from N/A to Medium.
sbassett moved this task from Watching to In Progress on the Security-Team board.Jun 7 2022, 3:18 AM
sbassett added a project: user-sbassett.
sbassett moved this task from Backlog to In Progress on the user-sbassett board.
sbassett added a subscriber: sbassett.EditedJun 7 2022, 3:31 AM

So the relevant logging action is in src/Backend/MWOAuthDataStore.php. It's using this oauth token format as part of the cache key, so technically nothing to do with query parameters. Since we're just worried about the logging action here, the simplest solution would be removing/redacting the oauth_token_secret value from the $key variable just prior to it getting logged. Testing within shell.php, something like:

$key = preg_replace("/(oauth_token_secret\=\w+:)/", "", $key);

or

$key = preg_replace("/(oauth_token_secret\=\w+:)/", "oauth_token_secret=[REDACTED]:", $key);

should work. Not sure if it's better to show the secret was redacted or have it disappear entirely, for those debugging with OAuth.log.

sbassett triaged this task as Medium priority.Jun 7 2022, 3:31 AM
sbassett added a project: Patch-For-Review.Jun 7 2022, 5:01 PM
sbassett added a subscriber: gerritbot.

Proposed patch below. Went with the redacted option to provide a small visual cue about what the cache key should look like.

01-T308861.patch1 KBDownload

sbassett changed the task status from Open to In Progress.Jun 7 2022, 5:02 PM
sbassett added subscribers: Reedy, aaron, Umherirrender, Tgr.
sbassett moved this task from In Progress to Security Patch To Deploy on the Security-Team board.Jun 8 2022, 3:10 AM
sbassett moved this task from Security Patch To Deploy to In Progress on the Security-Team board.Jun 27 2022, 9:08 PM
sbassett removed a project: Patch-For-Review.

Deployed to wmf.17: https://sal.toolforge.org/log/CVH9poEB6FQ6iqKillCF

Seems stable so far, I'll continue to monitor.

sbassett added a comment.Jun 28 2022, 3:32 PM

A quick tail of /srv/mw-log/OAuth.log on mwlog1002 today shows that the patch appears to be working as intended.

Reedy added a comment.Jun 28 2022, 3:53 PM

Just to note that patch won't pass PHPCS on CI ;)

sbassett added a comment.Jun 28 2022, 4:14 PM

Happy to eventually rework in gerrit and add that space as tribute to the style gods.

sbassett mentioned this in T311785: Write and send supplementary release announcement for extensions and skins with security patches (1.35.8/1.37.5/1.38.3).Jul 5 2022, 3:49 PM
thcipriani added a subscriber: thcipriani.Jul 28 2022, 9:38 PM

heads up: I made a minor change to this patch following this backport yesterday: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/OAuth/+/817860

sbassett added a comment.EditedAug 1 2022, 2:46 PM

Ok, thanks, @thcipriani. Updated patch here:

01-T308861.patch1 KBDownload

sbassett added a subscriber: RhinosF1.Aug 26 2022, 2:46 PM
RhinosF1 renamed this task from OAuth debug log includes consumer secrets to CVE-2022-39191: OAuth debug log includes consumer secrets.Sep 2 2022, 5:58 PM
mmartorana closed this task as Resolved.Oct 5 2022, 10:00 AM
mmartorana changed the visibility from "Custom Policy" to "Public (No Login Required)".
mmartorana changed the edit policy from "Custom Policy" to "All Users".
gerritbot added a comment.Oct 6 2022, 9:31 AM

Change 839459 had a related patch set uploaded (by Mmartorana; author: SBassett):

[mediawiki/extensions/OAuth@REL1_39] SECURITY: redact oauth_token_secret within log data

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

gerritbot added a project: Patch-For-Review.Oct 6 2022, 9:31 AM
gerritbot added a comment.Oct 6 2022, 9:36 AM

Change 839460 had a related patch set uploaded (by Mmartorana; author: SBassett):

[mediawiki/extensions/OAuth@master] SECURITY: redact oauth_token_secret within log data

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

gerritbot added a comment.Oct 6 2022, 10:05 AM

Change 839469 had a related patch set uploaded (by Mmartorana; author: SBassett):

[mediawiki/extensions/OAuth@REL1_38] SECURITY: redact oauth_token_secret within log data

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

gerritbot added a comment.Oct 6 2022, 10:08 AM

Change 839471 had a related patch set uploaded (by Mmartorana; author: SBassett):

[mediawiki/extensions/OAuth@REL1_37] SECURITY: redact oauth_token_secret within log data

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

gerritbot added a comment.Oct 6 2022, 10:10 AM

Change 839473 had a related patch set uploaded (by Mmartorana; author: SBassett):

[mediawiki/extensions/OAuth@REL1_35] SECURITY: redact oauth_token_secret within log data

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

gerritbot added a comment.Oct 6 2022, 3:20 PM

Change 839473 merged by jenkins-bot:

[mediawiki/extensions/OAuth@REL1_35] SECURITY: redact oauth_token_secret within log data

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

gerritbot added a comment.Oct 6 2022, 4:22 PM

Change 839460 merged by SBassett:

[mediawiki/extensions/OAuth@master] SECURITY: redact oauth_token_secret within log data

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

sbassett moved this task from In Progress to Our Part Is Done on the Security-Team board.Oct 6 2022, 4:34 PM
gerritbot added a comment.Oct 6 2022, 4:38 PM

Change 839471 merged by SBassett:

[mediawiki/extensions/OAuth@REL1_37] SECURITY: redact oauth_token_secret within log data

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

gerritbot added a comment.Oct 6 2022, 4:38 PM

Change 839459 merged by SBassett:

[mediawiki/extensions/OAuth@REL1_39] SECURITY: redact oauth_token_secret within log data

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

gerritbot added a comment.Oct 6 2022, 4:49 PM

Change 839469 merged by SBassett:

[mediawiki/extensions/OAuth@REL1_38] SECURITY: redact oauth_token_secret within log data

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

ReleaseTaggerBot added a project: MW-1.40-notes (1.40.0-wmf.5; 2022-10-10).Oct 6 2022, 5:00 PM
Octfx added a subscriber: Octfx.EditedOct 9 2022, 4:09 PM

This commit breaks OAuth on MW 1.35
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/OAuth/+/839473/2/src/Backend/MWOAuthDataStore.php#130

nonceCache is not defined, resulting in Call to a member function add() on null.

It works on 1.35+ as cache was renamed to ´nonceCache`

gerritbot added a comment.Oct 9 2022, 4:32 PM

Change 840668 had a related patch set uploaded (by Zabe; author: Zabe):

[mediawiki/extensions/OAuth@REL1_38] Fix cache var name

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

gerritbot added a comment.Oct 9 2022, 4:51 PM

Change 840668 merged by Zabe:

[mediawiki/extensions/OAuth@REL1_38] Fix cache var name

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

gerritbot added a comment.Oct 9 2022, 4:51 PM

Change 840302 had a related patch set uploaded (by Zabe; author: Zabe):

[mediawiki/extensions/OAuth@REL1_37] Fix cache var name

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

gerritbot added a comment.Oct 9 2022, 4:51 PM

Change 840303 had a related patch set uploaded (by Zabe; author: Zabe):

[mediawiki/extensions/OAuth@REL1_35] Fix cache var name

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

gerritbot added a comment.Oct 9 2022, 5:10 PM

Change 840303 merged by jenkins-bot:

[mediawiki/extensions/OAuth@REL1_35] Fix cache var name

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

gerritbot added a comment.Oct 9 2022, 5:11 PM

Change 840302 merged by Zabe:

[mediawiki/extensions/OAuth@REL1_37] Fix cache var name

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

sbassett added a subscriber: Zabe.Oct 11 2022, 8:01 PM
In T308861#8303090, @Octfx wrote:

This commit breaks OAuth on MW 1.35
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/OAuth/+/839473/2/src/Backend/MWOAuthDataStore.php#130

nonceCache is not defined, resulting in Call to a member function add() on null.

It works on 1.35+ as cache was renamed to ´nonceCache`

Yes, unfortunately I used the updated Wikimedia production patch for the supported release branch backports. Apologies for that. And thanks, @Zabe, for the quick fixes.

Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or 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