Development

#2844 ([Patch] sfEmailValidator validates invalid email addresses)

You must first sign up to be able to contribute.

Ticket #2844 (reopened defect)

Opened 4 months ago

Last modified 1 week ago

[Patch] sfEmailValidator validates invalid email addresses

Reported by: nicolas Assigned to: fabien
Priority: major Milestone:
Component: validation Version: 1.0.10
Keywords: email, regexp, sfEmailValidator Cc:
Qualification: Unreviewed

Description

I'm not an RFC purist nor expert, but I can't imagine that validating email addresses like <script>alert('t');</script>@toto.fr} is a good idea anyway :-)

So here's a revided regexp that prevent this kind of behavior:

Index: /Users/niko/www/symfonians/lib/symfony/validator/sfEmailValidator.class.php
===================================================================
--- /Users/niko/www/symfonians/lib/symfony/validator/sfEmailValidator.class.php	(revision 7171)
+++ /Users/niko/www/symfonians/lib/symfony/validator/sfEmailValidator.class.php	(working copy)
@@ -34,7 +34,7 @@
     $strict = $this->getParameterHolder()->get('strict');
     if ($strict == true)
     {
-      $re = '/^([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})$/i';
+      $re = '/^([a-zA-Z0-9_\.\-\+])+\@(([a-zA-Z0-9\-])+\.)+([a-zA-Z0-9]{2,4})+$/';
     }
     else
     {

Also, the problem remains with strict email validation set to false. I don't have a solution for the big Cal Henderson regexp :-(

Attachments

sfEmailValidator.patch (3.5 kB) - added by noel on 05/09/08 10:50:25.

Change History

01/26/08 18:16:51 changed by Andrejs.Verza

I have to warn, that the solution presented would eventually cause problems with emails having national characters in their names. Second, the problem does not lie in emails having JS in their names, but in the way how one presents the email (and(!) any other information) to user. The better idea is to use escaping - symfony's or manual, like htmlspecialchars. IMHO.

01/26/08 19:35:50 changed by nicolas

My main concern is that my postfix, respecting the RFC, won't send any email addresses formatted like that.

You're right regarding international chars in email addresses. Maybe an allow_unicode flag should allow some devs to take risks this way. Maybe just preventing < and > chars should do the trick. I don't know if it'd be sufficient.

At last, I never display email addresses, so it wasn't the aim of the patch. I just cannot imagine such emails can be validated. In any way :-)

01/26/08 21:07:54 changed by Andrejs.Verza

The validator's regexp, as it is now, seems a bit loose, indeed. I think it's about adopting the validator to the RFC rules (which I haven't studied), and I believe the RFC also should not get compromised by the validator. In any way! :-)

05/07/08 11:55:04 changed by noel

  • status changed from new to closed.
  • resolution set to fixed.
  • milestone set to 1.0.15.

in r8831 and r8833

05/08/08 13:55:21 changed by fabien

  • status changed from closed to reopened.
  • resolution deleted.
  • milestone deleted.

reverted

05/09/08 10:50:25 changed by noel

  • attachment sfEmailValidator.patch added.

05/09/08 10:51:28 changed by noel

I've tried to stick with the rfc2822, so what do you think about this patch?