Merge branch 'security/4.4.6-releng' into 4.4.6-releng

bestpractical · Jul 13, 2022 · cd685af · cd685af
2 parents 8cd02c8 + 024ce46
commit cd685af
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -96,6 +96,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);