From: Martin Renvoize Date: Fri, 22 Mar 2024 08:35:42 +0000 (+0000) Subject: Bug 1853138 - Switch to Email::Address::XS (#160) X-Git-Tag: bugzilla-5.2~32 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2002c3da428cfc5dc06ffff137e88dd33a08fa5a;p=thirdparty%2Fbugzilla.git Bug 1853138 - Switch to Email::Address::XS (#160) * Bug 1853138 - Switch to Email::Address::XS This patch updates us from using Email::Address to Email::Address::XS to resolve CVE-2015-7686. Regex comparisons have been replaced by parse + is_valid calls as per current documentation. Email::Address->parse($string) used to parse all addresses out of a general string, however Email::Address::XS expects the string to only contain addresses in the verious forms. This patch uses a basic regex to parse out strings that look like email addresses from the overall string passed, before passing to Email::Address::XS for validation and splitting out the 'host' part of the address. This patch restores the Email::Address::XS entry in Requirements.pm so that we can specify which version of the module we wish to use and make it explicit that we're using the module rather than relying on Email::Sender pulling it in for us. --- diff --git a/Bugzilla/Config/Common.pm b/Bugzilla/Config/Common.pm index 848b2ea7da..268333e4ee 100644 --- a/Bugzilla/Config/Common.pm +++ b/Bugzilla/Config/Common.pm @@ -11,7 +11,7 @@ use 5.14.0; use strict; use warnings; -use Email::Address; +use Email::Address::XS; use Socket; use Bugzilla::Util; @@ -80,7 +80,8 @@ sub check_regexp { sub check_email { my ($value) = @_; - if ($value !~ $Email::Address::mailbox) { + my ($mbox) = Email::Address::XS->parse($value); + if (!$mbox->is_valid) { return "must be a valid email address."; } return ""; diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index 6c05c4e897..0494f7d285 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -40,7 +40,7 @@ use Bugzilla::Error; use Bugzilla::Util; use Bugzilla::Group; -use Email::Address; +use Email::Address::XS; use List::MoreUtils qw(uniq); use base qw(Bugzilla::Object); @@ -306,12 +306,12 @@ sub _check_cc_list { || ThrowUserError('flag_type_cc_list_invalid', {cc_list => $cc_list}); my @addresses = split(/[,\s]+/, $cc_list); - 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 !~ /\P{ASCII}/ && $address =~ /^$addr_spec$/) + my $email = Email::Address::XS->parse_bare_address($address); + ($address !~ /\P{ASCII}/ && $email->is_valid) || ThrowUserError('illegal_email_address', {addr => $address, default => 1}); } return $cc_list; diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm index 3070ae36c3..3efd9c5d3c 100644 --- a/Bugzilla/Install/Requirements.pm +++ b/Bugzilla/Install/Requirements.pm @@ -121,8 +121,16 @@ sub REQUIRED_MODULES { # versions prior to 3.008 are broken, see https://bugzilla.mozilla.org/show_bug.cgi?id=1560873 {package => 'Template-Toolkit', module => 'Template', version => '3.008'}, - # 1.300011 has a debug mode for SMTP and automatically pass -i to sendmail. - {package => 'Email-Sender', module => 'Email::Sender', version => '1.300011',}, + # versions prior to 2.600 pulled Email::Address, we now use Email::Address::XS + {package => 'Email-Sender', module => 'Email::Sender', version => '2.600',}, + + # versions prior to 1.05 contain a security risk + { + package => 'Email-Address-XS', + module => 'Email::Address::XS', + version => '1.05', + }, + { package => 'Email-MIME', module => 'Email::MIME', diff --git a/Bugzilla/Migrate/Gnats.pm b/Bugzilla/Migrate/Gnats.pm index 1a2110c5c4..db47b94ce6 100644 --- a/Bugzilla/Migrate/Gnats.pm +++ b/Bugzilla/Migrate/Gnats.pm @@ -17,7 +17,7 @@ use Bugzilla::Constants; use Bugzilla::Install::Util qw(indicate_progress); use Bugzilla::Util qw(format_time trim generate_random_password); -use Email::Address; +use Email::Address::XS; use Email::MIME; use File::Basename; use IO::File; @@ -396,10 +396,10 @@ sub _get_gnats_field_data { if ($originator !~ Bugzilla->params->{emailregexp}) { # We use the raw header sometimes, because it looks like "From: user" - # which Email::Address won't parse but we can still use. + # which Email::Address::XS won't parse but we can still use. my $address = $email->header('From'); - my ($parsed) = Email::Address->parse($address); - if ($parsed) { + my ($parsed) = Email::Address::XS->parse($address); + if ($parsed->is_valid) { $address = $parsed->address; } if ($address) { diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index 0f786cef69..7e65e35c95 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -33,7 +33,7 @@ use Bugzilla::Error; use Date::Parse; use Date::Format; use Digest; -use Email::Address; +use Email::Address::XS; use List::Util qw(first); use Scalar::Util qw(tainted blessed); use Text::Wrap; @@ -223,13 +223,19 @@ sub html_light_quote { sub email_filter { my ($toencode) = @_; if (!Bugzilla->user->id) { - my @emails = Email::Address->parse($toencode); - if (scalar @emails) { - my @hosts = map { quotemeta($_->host) } @emails; - my $hosts_re = join('|', @hosts); - $toencode =~ s/\@(?:$hosts_re)//g; - return $toencode; + my $email_re = qr/([a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,})/; + my @hosts; + while ($toencode =~ /$email_re/g) { + my @emails = Email::Address::XS->parse($1); + if (scalar @emails) { + my @these_hosts = map { quotemeta($_->host) } @emails; + push @hosts, @these_hosts; + } } + my $hosts_re = join('|', @hosts); + + $toencode =~ s/\@(?:$hosts_re)//g; + return $toencode; } return $toencode; } @@ -740,17 +746,15 @@ sub validate_email_syntax { my $match = Bugzilla->params->{'emailregexp'}; 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. # We set the max length to 127 to ensure addresses aren't truncated when # inserted into the tokens.eventdata field. + my $address = Email::Address::XS->parse_bare_address($email); if ( $addr =~ /$match/ + && $address->is_valid && $email !~ /\P{ASCII}/ - && $email =~ /^$addr_spec$/ && length($email) <= 127) { # We assume these checks to suffice to consider the address untainted. diff --git a/email_in.pl b/email_in.pl index 5260a8b814..eecda81bf7 100755 --- a/email_in.pl +++ b/email_in.pl @@ -24,7 +24,7 @@ BEGIN { use lib qw(. lib); use Data::Dumper; -use Email::Address; +use Email::Address::XS; use Email::Reply qw(reply); use Email::MIME; use Getopt::Long qw(:config bundling); @@ -129,7 +129,7 @@ sub parse_mail { %fields = %{Bugzilla::Bug::map_fields(\%fields)}; - my ($reporter) = Email::Address->parse($input_email->header('From')); + my ($reporter) = Email::Address::XS->parse($input_email->header('From')); $fields{'reporter'} = $reporter->address; # The summary line only affects us if we're doing a post_bug.