Merge branch '4.4.6-releng' into 4.4-trunk

7 files changed, 59 insertions, 5 deletions

diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in
index d38b22b85f..84dc725d56 100644
--- a/etc/RT_Config.pm.in
+++ b/etc/RT_Config.pm.in
@@ -2590,6 +2590,18 @@ if there are other query arguments.
 
 Set( %ReferrerComponents );
 
+=item C<$StrictContentTypes>
+
+If set to 0, the C<X-Content-Type-Options: nosniff> header will be omitted on
+attachments.  Because RT does not filter HTML content in unknown content types,
+disabling this opens RT up to cross-site scripting (XSS) attacks by allowing
+the execution of arbitrary Javascript when the browser detects HTML-looking
+data in an attachment with an unknown content type.
+
+=cut
+
+Set($StrictContentTypes, 1);
+
 =item C<$BcryptCost>
 
 This sets the default cost parameter used for the C<bcrypt> key
diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm
index 0faad54131..76b72769c0 100644
--- a/lib/RT/ObjectCustomFieldValue.pm
+++ b/lib/RT/ObjectCustomFieldValue.pm
@@ -523,9 +523,9 @@ Get the OCFV cache key for this object
 
 sub GetOCFVCacheKey {
     my $self = shift;
-    my $ocfv_key = "CustomField-" . $self->CustomField
-        . '-ObjectType-' . $self->ObjectType
-        . '-ObjectId-' . $self->ObjectId;
+    my $ocfv_key = "CustomField-" . $self->__Value('CustomField')
+        . '-ObjectType-' . $self->__Value('ObjectType')
+        . '-ObjectId-' . $self->__Value('ObjectId');
     return $ocfv_key;
 }
 
@@ -806,6 +806,32 @@ sub ExternalStoreDigest {
     return $self->_Value( 'LargeContent' );
 }
 
+=head2 CurrentUserCanSee
+
+Returns true if user has "SeeCustomField" on the associated CustomField
+object, otherwise false.
+
+=cut
+
+sub CurrentUserCanSee {
+    my $self = shift;
+    return $self->CustomFieldObj->CurrentUserHasRight('SeeCustomField');
+}
+
+sub _Value {
+    my $self  = shift;
+    return undef unless $self->id;
+
+    unless ( $self->CurrentUserCanSee ) {
+        $RT::Logger->debug(
+            "Permission denied. User #". $self->CurrentUser->id
+            ." has no SeeCustomField right on CF #". $self->__Value('CustomField')
+        );
+        return undef;
+    }
+    return $self->SUPER::_Value(@_);
+}
+
 RT::Base->_ImportOverlays();
 
 1;
diff --git a/lib/RT/ObjectCustomFieldValues.pm b/lib/RT/ObjectCustomFieldValues.pm
index 83d603b2f3..54ff692c96 100644
--- a/lib/RT/ObjectCustomFieldValues.pm
+++ b/lib/RT/ObjectCustomFieldValues.pm
@@ -230,6 +230,15 @@ sub _DoCount {
     return $self->SUPER::_DoCount(@_);
 }
 
+
+sub AddRecord {
+    my $self = shift;
+    my ($record) = @_;
+
+    return unless $record->CurrentUserCanSee;
+    return $self->SUPER::AddRecord($record);
+}
+
 RT::Base->_ImportOverlays();
 
 # Clear the OCVF cache on exit to release connected RT::Ticket objects.
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 642e71632d..7189125a8b 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -2036,7 +2036,8 @@ sub _AddCustomFieldValue {
               );
         }
 
-        my $new_content = $new_value->Content;
+        # Fall back to '' in case current user doesn't have rights.
+        my $new_content = $new_value->Content // '';
 
         # For datetime, we need to display them in "human" format in result message
         #XXX TODO how about date without time?
diff --git a/lib/RT/System.pm b/lib/RT/System.pm
index 6d453c41fd..e79f7571da 100644
--- a/lib/RT/System.pm
+++ b/lib/RT/System.pm
@@ -386,7 +386,8 @@ sub ExternalStorageURLFor {
     # external storage direct links disabled
     return undef if !RT->Config->Get('ExternalStorageDirectLink');
 
-    return undef unless $Object->ContentEncoding eq 'external';
+    # If current user doesn't have rights, ContentEncoding is undef
+    return undef unless ( $Object->ContentEncoding // '' ) eq 'external';
 
     return $self->ExternalStorage->DownloadURLFor($Object);
 }
diff --git a/share/html/Download/CustomFieldValue/dhandler b/share/html/Download/CustomFieldValue/dhandler
index 5f0857e346..5099299531 100644
--- a/share/html/Download/CustomFieldValue/dhandler
+++ b/share/html/Download/CustomFieldValue/dhandler
@@ -61,6 +61,8 @@ unless ($OCFV->id) {
     Abort("Bad OCFV id. Couldn't find OCFV '$id'\n");
 }
 
+Abort( loc('Permission Denied'), Code => HTTP::Status::HTTP_FORBIDDEN ) unless $OCFV->CurrentUserCanSee;
+
 my $content_type = $OCFV->ContentType || 'text/plain; charset=utf-8';
     
 if (RT->Config->Get('AlwaysDownloadAttachments')) {
@@ -70,6 +72,7 @@ elsif (!RT->Config->Get('TrustHTMLAttachments')) {
     $content_type = 'text/plain; charset=utf-8' if ($content_type =~ /^text\/html/i);
 }
 
+$r->headers_out->{'X-Content-Type-Options'} = 'nosniff' if RT->Config->Get('StrictContentTypes');
 $r->content_type( $content_type );
 $m->clear_buffer();
 $m->out($OCFV->LargeContent);
diff --git a/share/html/Ticket/Attachment/dhandler b/share/html/Ticket/Attachment/dhandler
index 1a94239d96..045896aaed 100644
--- a/share/html/Ticket/Attachment/dhandler
+++ b/share/html/Ticket/Attachment/dhandler
@@ -83,6 +83,7 @@ elsif (lc $content_type eq 'text/html') {
         Content     => \$content,
         Attachment  => $AttachmentObj,
     );
+    $count //= 0;  # RewriteInlineImages can return undef when nothing needs to be rewritten
     RT->Logger->debug("Rewrote $count CID images when displaying original HTML attachment #$attach");
 }
 
@@ -96,6 +97,7 @@ unless ( $mimetype && $mimetype->isBinary ) {
     $content_type .= ";charset=$iana";
 }
 
+$r->headers_out->{'X-Content-Type-Options'} = 'nosniff' if RT->Config->Get('StrictContentTypes');
 $r->content_type($content_type);
 $m->clear_buffer();
 $m->out($content);