Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security: more trimmed/mapped chars in user names #11

Merged
merged 3 commits into from Dec 20, 2020

Conversation

leaf-node
Copy link

Hi @jpgill86

I have a tweak to the security patch that you merged last week. It blocks additional characters that I found are mapped or stripped from user names. The regexes are based on upstream MediaWiki code.

https://github.com/wikimedia/mediawiki/blob/master/includes/title/MediaWikiTitleCodec.php#L314-L328

I also wrote code to reject user names with invalid Unicode, and to not show a stack trace if MediaWiki itself rejects a user name.

I tested these changes on two wikis. Is this something that you are willing to review and merge?

Thanks, : )
Andrew

I found more characters in MediaWiki that are mapped to other chars, or
trimmed from user names. They mostly include bidirectional override
symbols and various forms of blank space.

Blocking these symbols in user names makes it harder to masquerade as a
bureaucrat user.
MediaWiki already seems to reject user names that contain invalid
Unicode characters when logging in. However, this code also rejects such
user names, mimicing the way that the `splitTitleString` method trims
invalid characters from user names.
If MediaWiki rejects a user name, `User::newFromName` returns false.
Follow the normal procedure for rejecting a login.
@jpgill86
Copy link
Member

Same disclaimer as last time, but it seems OK to me. I will merge. Thanks again!

@jpgill86 jpgill86 merged commit 9db3ebb into CWRUChielLab:master Dec 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants