From ddb3ab99a6eb359394a6d9c9b5ec4d471c061601 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Mon, 14 May 2012 15:21:11 -0400 Subject: [PATCH] AddAttachments must use RT->SystemUser when searching for attachments 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. --- lib/RT/Action/SendEmail.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/RT/Action/SendEmail.pm b/lib/RT/Action/SendEmail.pm index 94686b894e4..4ae1a8b66d6 100644 --- a/lib/RT/Action/SendEmail.pm +++ b/lib/RT/Action/SendEmail.pm @@ -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