Change in core module security risk

Question

As interim solution to an issue in JavaScript, I changed the User module code, to disallow single quotation mark. I changed the following line.

if (preg_match('/[^\x{80}-\x{F7} a-z0-9@_.\'-]/i', $name)) {

After my edit, it becomes as follows.

if (preg_match('/[^\x{80}-\x{F7} a-z0-9@_.-]/i', $name)) {

One of our senior Drupal developers said that this is not an acceptable solution because it could open up the site to serious security risks and it could potentially lead to a security flaw around form injection.

I do not understand why that could happen. Can anyone explain me why?

PS. I understand that this will get overwritten on Drupal update. This was to temporarily prevent serious bug that is blocking the whole page from functioning.

Answer 1

So it turns out the regexp is not for avoiding apostrophes but to allow them, you can track down the origin to this issue to 6 years ago: https://drupal.org/node/266488

I've added apostrophe support(+test) and a small change in check_plain() as proposed in http://drupal.org/node/165226 in the new version of the patch.

So it would be not due a security flaw but to have names with apostrophes correctly handled as you can see in the issue above:

One of my friends is fortunate, or unfortunate, enough to have an apostrophe in his last name. Many first and last names have this, for example: O’Reilly.

The crazy thing is: Drupal completely ignores this, and O'Reilly becomes OReily

The reason why this is crashing your site is hard to foresee, I stand that you might be fixing the symptoms here and not the cause by changing the regexp. Check the username databse you've got for strange chars.

If you want to have a way to modify this validation pattern, you can check this module Custom username validation.