Skip to content
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

Fix #533 — Prevent the use of the list address as subscriber #1366

Merged
merged 4 commits into from
Mar 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions default/mail_tt2/report.tt2
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,15 @@ Warning: this message may already have been sent by one of the list's moderators
[%~ ELSIF report_entry == 'blocklisted_domain' ~%]
[%|loc(report_param.email || report_param.value)%]Address "%1" belongs to a blocklisted domain[%END%]

[%~ ELSIF report_entry == 'email_is_the_list' ~%]
[% IF report_param.role == 'owner' ~%]
[%|loc(report_param.email)%]Address "%1" is the address for the list owners[%END%]
[%~ ELSIF report_param.role == 'editor' ~%]
[%|loc(report_param.email)%]Address "%1" is the address for the list moderators[%END%]
[%~ ELSE ~%]
[%|loc(report_param.email)%]Address "%1" is the address of the list[%END%]
[%~ END %]

[%~ ELSIF report_entry == 'incorrect_passwd' ~%]
[%|loc%]Provided password is incorrect[%END%]

Expand Down
34 changes: 31 additions & 3 deletions src/lib/Sympa/List.pm
Original file line number Diff line number Diff line change
Expand Up @@ -3117,13 +3117,25 @@ sub add_list_member {
unless (Sympa::Tools::Text::valid_email($u->{email})) {
$log->syslog('err', 'Ignoring %s which is not a valid email',
$u->{email});
push @$stash_ref,
[
'user', 'incorrect_email',
{email => $u->{email}, role => 'member'}
];
next;
}

my $who = Sympa::Tools::Text::canonic_email($u->{email});
if (Sympa::Tools::Domains::is_blocklisted($who)) {
$log->syslog('err', 'Ignoring %s which uses a blocklisted domain',
$u->{email});
$who);
push @$stash_ref, ['user', 'blocklisted_domain', {email => $who}];
next;
}
if ($who eq $self->get_id) {
$log->syslog('err',
'Ignoring %s which is the address of the list', $who);
push @$stash_ref, ['user', 'email_is_the_list', {email => $who}];
next;
}
unless (
Expand Down Expand Up @@ -3314,7 +3326,7 @@ sub add_list_admin {
$sdm->begin;

foreach my $user (@users) {
$total++ if $self->_add_list_admin($role, $user, stash => $stash_ref);
$total++ if $self->_add_list_admin($role, $user, %options);
}

unless ($sdm->commit) {
Expand All @@ -3338,9 +3350,25 @@ sub _add_list_admin {

my $stash_ref = $options{stash} || [];

return undef unless Sympa::Tools::Text::valid_email($u->{email});
unless (Sympa::Tools::Text::valid_email($u->{email})) {
$log->syslog('err', 'Ignoring %s which is not a valid email',
$u->{email});
push @$stash_ref,
['user', 'incorrect_email',
{email => $u->{email}, role => $role}];
return undef;
}
my $who = Sympa::Tools::Text::canonic_email($u->{email});

if ($who eq Sympa::get_address($self, $role)) {
$log->syslog('err',
'Ignoring %s which is the address for the list %s',
$who, $role);
push @$stash_ref,
['user', 'email_is_the_list', {email => $who, role => $role}];
return undef;
}

my $values = $self->get_default_user_options(role => $role);
while (my ($k, $v) = each %$u) {
$values->{$k} = $v if defined $v;
Expand Down
23 changes: 2 additions & 21 deletions src/lib/Sympa/Request/Handler/add.pm
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ use Sympa;
use Conf;
use Sympa::Language;
use Sympa::Log;
use Sympa::Tools::Domains;
use Sympa::Tools::Password;
use Sympa::Tools::Text;
use Sympa::User;

use base qw(Sympa::Request::Handler);
Expand Down Expand Up @@ -66,31 +63,15 @@ sub _twist {

$language->set_lang($list->{'admin'}{'lang'});

unless (Sympa::Tools::Text::valid_email($email)) {
$self->add_stash($request, 'user', 'incorrect_email',
{'email' => $email});
$log->syslog('err',
'request "add" rejected; incorrect email "%s"', $email);
return undef;
}

my @stash;
if ($role eq 'member') {
unless ($request->{force} or $list->is_subscription_allowed) {
$log->syslog('info', 'List %s not open', $list);
$self->add_stash($request, 'user', 'list_not_open',
{'status' => $list->{'admin'}{'status'}});
{status => $list->{'admin'}{'status'}});
$self->{finish} = 1;
return undef;
}
if (Sympa::Tools::Domains::is_blocklisted($email)) {
$self->add_stash($request, 'user', 'blocklisted_domain',
{'email' => $email});
$log->syslog('err',
'request "add" rejected; blocklisted domain for "%s"',
$email);
return undef;
}

$list->add_list_member(
{email => $email, gecos => $comment, custom_attribute => $ca},
Expand Down Expand Up @@ -136,7 +117,7 @@ sub _report_member {
my $comment = $request->{gecos};

$self->add_stash($request, 'notice', 'now_subscriber',
{'email' => $email, listname => $list->{'name'}});
{email => $email, listname => $list->{'name'}});

# FIXME: Required?
my $user = Sympa::User->new($email);
Expand Down
2 changes: 1 addition & 1 deletion src/lib/Sympa/Request/Handler/del.pm
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ sub _twist {
unless ($request->{force} or $list->is_subscription_allowed) {
$log->syslog('info', 'List %s not open', $list);
$self->add_stash($request, 'user', 'list_not_open',
{'status' => $list->{'admin'}{'status'}});
{status => $list->{'admin'}{'status'}});
$self->{finish} = 1;
return undef;
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/Sympa/Request/Handler/include.pm
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,8 @@ sub _update_users {
while (my $entry = $ds->next) {
my $email = $entry->[0];

next unless Sympa::Tools::Text::valid_email($email);
$email = Sympa::Tools::Text::canonic_email($email);
$email = Sympa::Tools::Text::canonic_email($email)
if Sympa::Tools::Text::valid_email($email);

# 1. If role of the data source is 'member' and the user is excluded:
# Do nothing.
Expand Down
9 changes: 3 additions & 6 deletions src/lib/Sympa/Request/Handler/signoff.pm
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,10 @@ sub _twist {

# If a list is not 'open' and allow_subscribe_if_pending has been set to
# 'off' returns undef.
unless ($list->{'admin'}{'status'} eq 'open'
or
Conf::get_robot_conf($list->{'domain'}, 'allow_subscribe_if_pending')
eq 'on') {
$self->add_stash($request, 'user', 'list_not_open',
{'status' => $list->{'admin'}{'status'}});
unless ($list->is_subscription_allowed) {
$log->syslog('info', 'List %s not open', $list);
$self->add_stash($request, 'user', 'list_not_open',
{status => $list->{'admin'}{'status'}});
return undef;
}

Expand Down
21 changes: 4 additions & 17 deletions src/lib/Sympa/Request/Handler/subscribe.pm
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ use Sympa;
use Conf;
use Sympa::Language;
use Sympa::Log;
use Sympa::Tools::Domains;
use Sympa::Tools::Password;
use Sympa::User;

use base qw(Sympa::Request::Handler);
Expand Down Expand Up @@ -74,24 +72,13 @@ sub _twist {
undef $comment;
}

if (Sympa::Tools::Domains::is_blocklisted($email)) {
$self->add_stash($request, 'user', 'blocklisted_domain',
{'email' => $email});
$log->syslog('err',
'SUBSCRIBE to %s command rejected; blocklisted domain for "%s"',
$list, $email);
return undef;
}

# If a list is not 'open' and allow_subscribe_if_pending has been set to
# 'off' returns undef.
unless ($list->{'admin'}{'status'} eq 'open'
or
Conf::get_robot_conf($list->{'domain'}, 'allow_subscribe_if_pending')
eq 'on') {
$self->add_stash($request, 'user', 'list_not_open',
{'status' => $list->{'admin'}{'status'}});
unless ($list->is_subscription_allowed) {
$log->syslog('info', 'List %s not open', $list);
$self->add_stash($request, 'user', 'list_not_open',
{status => $list->{'admin'}{'status'}});
$self->{finish} = 1;
return undef;
}

Expand Down