Skip to content

Commit

Permalink
Prevent email about private note to unprivileged users
Browse files Browse the repository at this point in the history
In email_collect_recipient(), the logic to exclude users who can't see
bugnotes relied on comparing the issue's last updated timestamp with the
bugnote's date.

Since these dates are not necessarily equal as they are updated
separately when a bugnote is added, this may result in a race condition
causing a notification e-mail about a new private bugnote to be sent to
users not authorized to see them.

Since email_collect_recipient()'s $p_bugnote_id parameter is always null
except for 'bugnote' notifications, the date check is not necessary; it
is sufficient to check that $p_bugnote_id is not null.

Fixes #22898
  • Loading branch information
dregad committed Aug 25, 2019
1 parent 380fc71 commit ad42c3c
Showing 1 changed file with 9 additions and 10 deletions.
19 changes: 9 additions & 10 deletions core/email_api.php
Original file line number Diff line number Diff line change
Expand Up @@ -331,13 +331,6 @@ function email_collect_recipients( $p_bug_id, $p_notify_type, array $p_extra_use
}

# add users who contributed bugnotes
$t_bugnote_id = ( $p_bugnote_id === null ) ? bugnote_get_latest_id( $p_bug_id ) : $p_bugnote_id;
if( $t_bugnote_id !== 0 ) {
$t_bugnote_date = bugnote_get_field( $t_bugnote_id, 'last_modified' );
}
$t_bug = bug_get( $p_bug_id );
$t_bug_date = $t_bug->last_updated;

$t_notes_enabled = ( ON == email_notify_flag( $p_notify_type, 'bugnotes' ) );
db_param_push();
$t_query = 'SELECT DISTINCT reporter_id FROM {bugnote} WHERE bug_id = ' . db_param();
Expand Down Expand Up @@ -393,6 +386,9 @@ function email_collect_recipients( $p_bug_id, $p_notify_type, array $p_extra_use
case 'closed':
case 'bugnote':
$t_pref_field = 'email_on_' . $p_notify_type;
if( !$p_bugnote_id ) {
$p_bugnote_id = bugnote_get_latest_id( $p_bug_id );
}
break;
case 'owner':
# The email_on_assigned notification type is now effectively
Expand All @@ -419,6 +415,7 @@ function email_collect_recipients( $p_bug_id, $p_notify_type, array $p_extra_use
# of user ids so we could pull them all in. We'll see if it's necessary
$t_final_recipients = array();

$t_bug = bug_get( $p_bug_id );
$t_user_ids = array_keys( $t_recipients );
user_cache_array_rows( $t_user_ids );
user_pref_cache_array_rows( $t_user_ids );
Expand Down Expand Up @@ -461,9 +458,11 @@ function email_collect_recipients( $p_bug_id, $p_notify_type, array $p_extra_use

# exclude users who don't have at least viewer access to the bug,
# or who can't see bugnotes if the last update included a bugnote
if( !access_has_bug_level( config_get( 'view_bug_threshold', null, $t_id, $t_bug->project_id ), $p_bug_id, $t_id )
|| ( $t_bugnote_id !== 0 &&
$t_bug_date == $t_bugnote_date && !access_has_bugnote_level( config_get( 'view_bug_threshold', null, $t_id, $t_bug->project_id ), $t_bugnote_id, $t_id ) )
$t_view_bug_threshold = config_get( 'view_bug_threshold', null, $t_id, $t_bug->project_id );
if( !access_has_bug_level( $t_view_bug_threshold, $p_bug_id, $t_id )
|| ( $p_bugnote_id
&& !access_has_bugnote_level( $t_view_bug_threshold, $p_bugnote_id, $t_id )
)
) {
log_event( LOG_EMAIL_RECIPIENT, 'Issue = #%d, drop @U%d (access level)', $p_bug_id, $t_id );
continue;
Expand Down

0 comments on commit ad42c3c

Please sign in to comment.