Skip to content

[8.x] Add Transliterate shortcut to the Str helper #40681

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

Merged
merged 4 commits into from
Jan 28, 2022

Conversation

liamh101
Copy link
Contributor

Description

Add a transliterate shortcut to the String helper.

I've added the by recommendation on a recent PR around fixing security exploit in the UI package.

In short, MYSQL and other DB hosts will transliterate special characters in queries, making it possible to bypass security checks and throttles.

In the case of the UI security exploit, by default, Laravel throttles login attempts by the applications email/username. However, if I use special characters to replace standard characters in my email when the email gets to the Database, the query parameters will transliterate. This means that ⓣⓔⓢⓣ@ⓛⓐⓡⓐⓥⓔⓛ.ⓒⓞⓜ will be read as [email protected] by the Database. This made it was possible for a user to brute force a password by bypassing the login throttle.

Adding this shortcut will make it easier to implement a more elegant solution to the current UI fix while also making it easier for users to improve the security of their application or similar functionality by adding transliteration directly from the helper without needing prior knowledge of the ASCII package.

@GrahamCampbell GrahamCampbell changed the title Add Transliterate shortcut to the Str helper [8.x] Add Transliterate shortcut to the Str helper Jan 27, 2022
@taylorotwell
Copy link
Member

Some email addresses could genuinely have UTF-8 characters in them though, right? Or is that not allowed? 😅

* @param bool|null $strict
* @return string
*/
public static function transliterate($string, $unkown = '?', $strict = false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the $unkown argument be $unknown? https://github.com/voku/portable-ascii/blob/f1c1181c85e099b66713c7a36a6d65814795d149/src/voku/helper/ASCII.php#L1178

If accepted, most Str methods also get added to the Stringable class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I debated changing this and the strict flag. As both on their own are nondescript. However, decided to keep their original names for consistency.

I could change $unkown to $unkownFallback and $strict to $useIntl as it's a bit more descriptive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I meant that there's a typo missing the second 'n' in unk_n_own.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D'oh! Two secs

@liamh101
Copy link
Contributor Author

Some email addresses could genuinely have UTF-8 characters in them though, right? Or is that not allowed? 😅

Currently, the only allowed special characters are ~!$%^&*_=+}{'?-. however for the exploit it doesn't really matter as that email is just generating a key to store against. So thankfully if emoji emails do become a thing, that fix will still hold up... Mostly 😛

Liam Hackett and others added 2 commits January 27, 2022 22:01
@taylorotwell taylorotwell merged commit f89fb45 into laravel:8.x Jan 28, 2022
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.

3 participants