Bug 319953: Missing real email syntax check

r=glob a=LpSolit

3 files changed, 31 insertions, 12 deletions

diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm
index ddaa6eb62..b4709212e 100644
--- a/Bugzilla/FlagType.pm
+++ b/Bugzilla/FlagType.pm
@@ -38,6 +38,8 @@ use Bugzilla::Error;
 use Bugzilla::Util;
 use Bugzilla::Group;
 
+use Email::Address;
+
 use base qw(Bugzilla::Object);
 
 ###############################
@@ -287,15 +289,11 @@ sub _check_cc_list {
       || ThrowUserError('flag_type_cc_list_invalid', { cc_list => $cc_list });
 
     my @addresses = split(/[,\s]+/, $cc_list);
-    # We do not call Util::validate_email_syntax because these
-    # addresses do not require to match 'emailregexp' and do not
-    # depend on 'emailsuffix'. So we limit ourselves to a simple
-    # sanity check:
-    # - match the syntax of a fully qualified email address;
-    # - do not contain any illegal character.
+    my $addr_spec = $Email::Address::addr_spec;
+    # We do not call check_email_syntax() because these addresses do not
+    # require to match 'emailregexp' and do not depend on 'emailsuffix'.
     foreach my $address (@addresses) {
-        ($address =~ /^[\w\.\+\-=]+@[\w\.\-]+\.[\w\-]+$/
-           && $address !~ /[\\\(\)<>&,;:"\[\] \t\r\n]/)
+        ($address !~ /\P{ASCII}/ && $address =~ /^$addr_spec$/)
           || ThrowUserError('illegal_email_address',
                             {addr => $address, default => 1});
     }
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 373fc1ef3..23b08c63a 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -195,8 +195,7 @@ sub check_login_name_for_creation {
     my ($invocant, $name) = @_;
     $name = trim($name);
     $name || ThrowUserError('user_login_required');
-    validate_email_syntax($name)
-        || ThrowUserError('illegal_email_address', { addr => $name });
+    check_email_syntax($name);
 
     # Check the name if it's a new user, or if we're changing the name.
     if (!ref($invocant) || $invocant->login ne $name) {
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index a04095647..bf8569839 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -20,7 +20,7 @@ use base qw(Exporter);
                              format_time validate_date validate_time datetime_from
                              file_mod_time is_7bit_clean
                              bz_crypt generate_random_password
-                             validate_email_syntax clean_text
+                             validate_email_syntax check_email_syntax clean_text
                              get_text template_var disable_utf8
                              detect_encoding);
 
@@ -552,7 +552,13 @@ sub generate_random_password {
 sub validate_email_syntax {
     my ($addr) = @_;
     my $match = Bugzilla->params->{'emailregexp'};
-    my $ret = ($addr =~ /$match/ && $addr !~ /[\\\(\)<>&,;:"\[\] \t\r\n]/);
+    my $email = $addr . Bugzilla->params->{'emailsuffix'};
+    # This regexp follows RFC 2822 section 3.4.1.
+    my $addr_spec = $Email::Address::addr_spec;
+    # RFC 2822 section 2.1 specifies that email addresses must
+    # be made of US-ASCII characters only.
+    # Email::Address::addr_spec doesn't enforce this.
+    my $ret = ($addr =~ /$match/ && $email !~ /\P{ASCII}/ && $email =~ /^$addr_spec$/);
     if ($ret) {
         # We assume these checks to suffice to consider the address untainted.
         trick_taint($_[0]);
@@ -560,6 +566,15 @@ sub validate_email_syntax {
     return $ret ? 1 : 0;
 }
 
+sub check_email_syntax {
+    my ($addr) = @_;
+
+    unless (validate_email_syntax(@_)) {
+        my $email = $addr . Bugzilla->params->{'emailsuffix'};
+        ThrowUserError('illegal_email_address', { addr => $email });
+    }
+}
+
 sub validate_date {
     my ($date) = @_;
     my $date2;
@@ -763,6 +778,7 @@ Bugzilla::Util - Generic utility functions for bugzilla
 
   # Validation Functions
   validate_email_syntax($email);
+  check_email_syntax($email);
   validate_date($date);
 
   # DB-related functions
@@ -1069,6 +1085,12 @@ Do a syntax checking for a legal email address and returns 1 if
 the check is successful, else returns 0.
 Untaints C<$email> if successful.
 
+=item C<check_email_syntax($email)>
+
+Do a syntax checking for a legal email address and throws an error
+if the check fails.
+Untaints C<$email> if successful.
+
 =item C<validate_date($date)>
 
 Make sure the date has the correct format and returns 1 if