Page MenuHomePhabricator
Create Task
Maniphest T115522

Passwords generated by User::randomPassword() may be shorter than $wgMinimalPasswordLength
Closed, ResolvedPublic
Actions

Assigned To
csteipp
Authored By
Frankrfarmer
Oct 14 2015, 9:16 PM
Tags
Referenced Files
F2724475: 00d862b.patch
Oct 15 2015, 7:20 PM
F2721943: 0001-0-pad-to-length-in-random-string-generation-for-backporting.patch
Oct 14 2015, 9:54 PM
F2721927: 0001-0-pad-to-length-in-random-string-generation.patch
Oct 14 2015, 9:48 PM
Unknown Object (File)
Oct 14 2015, 9:46 PM
Subscribers
csteipp
demon
Frankrfarmer
gerritbot
Grunny
Luke081515

Description

Some quick testing of the output of User::randomPassword() shows that by default output is generated in the range 0 to 7vvvvvvvvv. With a default configuration all passwords should be 10 characters; instead passwords of 9 or fewer characters are easily generated every few thousand invocations of the method -- in very rare cases, the method might generate a password just one single character long.

Ideally, this method should always return $wgMinimalPasswordLength characters (10 by default) -- in the range 0000000000 to vvvvvvvvvv

patches:

  • master -
    0001-0-pad-to-length-in-random-string-generation.patch1 KBDownload
  • 1.23 - 1.26 -
    0001-0-pad-to-length-in-random-string-generation-for-backporting.patch1 KBDownload

CVE: CVE-2015-8626

Details

ProjectBranchLines +/-Subject
mediawiki/coremaster+2 -3[SECURITY] 0-pad to length in random string generation
mediawiki/coreREL1_26+2 -3[SECURITY] 0-pad to length in random string generation
mediawiki/coreREL1_25+2 -3[SECURITY] 0-pad to length in random string generation
mediawiki/coreREL1_23+2 -3[SECURITY] 0-pad to length in random string generation
mediawiki/coreREL1_24+2 -3[SECURITY] 0-pad to length in random string generation
Customize query in gerrit

Related Objects
Search...

Event Timeline

Maniphest changed the visibility from "Public (No Login Required)" to "Custom Policy".Oct 14 2015, 9:16 PM
Maniphest changed the edit policy from "All Users" to "Custom Policy".
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 14 2015, 9:16 PM
Frankrfarmer created this task.Oct 14 2015, 9:16 PM
Frankrfarmer updated the task description. (Show Details)
Frankrfarmer added projects: MediaWiki-Core-AuthManager, acl*security.
Frankrfarmer changed Security from None to Software security bug.
Frankrfarmer edited subscribers, added: Frankrfarmer; removed: Aklapper.
Anomie added a subscriber: Anomie.EditedOct 14 2015, 9:46 PM

Patches for security bugs should not be posted publicly!

An easier fix is to just pass the padding length into wfBaseConvert, as shown here.

0001-0-pad-to-length-in-random-string-generation.patch1 KBDownload

While this won't apply to previous versions of MediaWiki, the same change can easily be applied to User.php:

0001-0-pad-to-length-in-random-string-generation-for-backporting.patch1 KBDownload

Frankrfarmer added a comment.Oct 15 2015, 7:20 PM

Sorry, let me upload the patch directly here, and remove it from github:

00d862b.patch2 KBDownload

Of course your version is more than sufficient as well

Frankrfarmer updated the task description. (Show Details)Oct 15 2015, 7:22 PM
csteipp triaged this task as Low priority.Oct 20 2015, 12:26 AM
csteipp added a subscriber: csteipp.
In T115522#1726561, @Anomie wrote:

0001-0-pad-to-length-in-random-string-generation.patch1 KBDownload

Patch looks good to me. We'll get this deployed.

csteipp closed this task as Resolved.Nov 2 2015, 7:07 PM
csteipp claimed this task.
csteipp mentioned this in T97897: Incorrect parsing of IPs for global block.
csteipp added a parent task: T115722: MediaWiki Security release 1.25.4.

18:53 csteipp: deployed patches for T97897 & T115522

demon added a subscriber: demon.Dec 15 2015, 8:08 PM

While this won't apply to previous versions of MediaWiki, the same change can easily be applied to User.php:

0001-0-pad-to-length-in-random-string-generation-for-backporting.patch1 KBDownload

This doesn't apply to 1.26, 1.25 or 1.24 as this was already looking ok in those branches afaict. I think this is a regression only affecting 1.23 and master.

csteipp added a comment.Dec 15 2015, 8:26 PM

Looks like it applies to 1.24, 1.25, and 1.26 cleanly, and is needed.

Anomie added a comment.Dec 15 2015, 8:31 PM
In T115522#1882054, @demon wrote:

While this won't apply to previous versions of MediaWiki, the same change can easily be applied to User.php:

0001-0-pad-to-length-in-random-string-generation-for-backporting.patch1 KBDownload

This doesn't apply to 1.26, 1.25 or 1.24

It does when I try it (with git am). I'm currently seeing head for 1.26 is 79902bb6, 1.25 is 1c4d1ee5, and 1.24 is f8d608e6.

as this was already looking ok in those branches afaict. I think this is a regression only affecting 1.23 and master.

All versions since r114233 appear to have been affected.

demon added a comment.Dec 15 2015, 8:46 PM

Bleh this was user error, ignore me

demon added a subscriber: Grunny.Dec 17 2015, 1:19 AM
demon changed the visibility from "Custom Policy" to "Public (No Login Required)".Dec 18 2015, 12:40 AM
demon changed the edit policy from "Custom Policy" to "All Users".
demon changed Security from Software security bug to None.
gerritbot added a subscriber: gerritbot.Dec 18 2015, 12:40 AM

Change 259949 had a related patch set uploaded (by Chad):
[SECURITY] 0-pad to length in random string generation

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

gerritbot added a project: Patch-For-Review.Dec 18 2015, 12:40 AM
gerritbot added a comment.Dec 18 2015, 1:04 AM

Change 259914 merged by Chad:
[SECURITY] 0-pad to length in random string generation

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

gerritbot added a comment.Dec 18 2015, 1:31 AM

Change 259903 merged by Reedy:
[SECURITY] 0-pad to length in random string generation

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

gerritbot added a comment.Dec 18 2015, 1:31 AM

Change 259938 merged by Chad:
[SECURITY] 0-pad to length in random string generation

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

gerritbot added a comment.Dec 18 2015, 1:34 AM

Change 259927 merged by Reedy:
[SECURITY] 0-pad to length in random string generation

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

ReleaseTaggerBot added projects: MW-1.25-release, MW-1.24-release, MW-1.23-release, MW-1.26-release.Dec 18 2015, 2:00 AM
gerritbot added a comment.Dec 18 2015, 9:35 AM

Change 259949 merged by jenkins-bot:
[SECURITY] 0-pad to length in random string generation

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

ReleaseTaggerBot added projects: MW-1.27-release (WMF-deploy-2016-01-12_(1.27.0-wmf.10)), MW-1.27-release-notes.Dec 18 2015, 10:00 AM
csteipp updated the task description. (Show Details)Dec 24 2015, 12:09 AM
csteipp added a project: Vuln-Authn/Session.
Restricted Application added a subscriber: Luke081515. · View Herald TranscriptDec 24 2015, 12:09 AM
• chasemp added a project: Security.Feb 10 2020, 11:00 PM
• chasemp removed a project: acl*security.Feb 20 2020, 8:18 PM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:43 PM
Content licensed under Creative Commons Attribution-ShareAlike 4.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