Page MenuHomePhabricator

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

Description

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

Event Timeline

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 added a project: user-sbassett.
sbassett moved this task from Backlog to In Progress on the user-sbassett board.

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 subscriber: gerritbot.

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

sbassett changed the task status from Open to In Progress.Jun 7 2022, 5:02 PM
sbassett added subscribers: Reedy, aaron, Umherirrender, Tgr.
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.

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

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

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

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

Ok, thanks, @thcipriani. Updated patch here:

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 changed the visibility from "Custom Policy" to "Public (No Login Required)".
mmartorana changed the edit policy from "Custom Policy" to "All Users".

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

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

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

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

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

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

Change 839460 merged by SBassett:

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

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

Change 839471 merged by SBassett:

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

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

Change 839459 merged by SBassett:

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

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

Change 839469 merged by SBassett:

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

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

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`

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

Change 840668 merged by Zabe:

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

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

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

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

Change 840303 merged by jenkins-bot:

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

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

Change 840302 merged by Zabe:

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

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

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.