Skip to content

Commit

Permalink
AddAttachments must use RT->SystemUser when searching for attachments…
Browse files Browse the repository at this point in the history
… to use

c29107c changed AddAttachments to use the transaction's current user to
search for which attachments to add to the outgoing mail.
Unfortunately, this ignored the common case where the transaction's
current user is an unprivileged user who does not have rights to see
their own attachment.  This manifested itself as AdminCc emails not
having attachments which were included with the original mail that
triggered them, despite RT-Attach-Message being set.

Revert the CurrentUser on the Attachments search to RT->SystemUser, as
it was pre- c29107c.  This does not re-open the vulnerability, as
(unlike the AddTicket functionality) the transaction creator can only
cause attachments on their own transaction to be distributed.  While one
route to fix this would be to modify RT::Attachments->Next to allow
creators to always see their own attachments, such a change might have
broader-reaching implications.
  • Loading branch information
alexmv committed May 14, 2012
1 parent 299b660 commit ddb3ab9
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion lib/RT/Action/SendEmail.pm
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ sub AddAttachments {

$MIMEObj->head->delete('RT-Attach-Message');

my $attachments = RT::Attachments->new( $self->TransactionObj->CreatorObj );
my $attachments = RT::Attachments->new( RT->SystemUser );
$attachments->Limit(
FIELD => 'TransactionId',
VALUE => $self->TransactionObj->Id
Expand Down

0 comments on commit ddb3ab9

Please sign in to comment.