From 7986fd798df5d055ea2ff9f74207631ab307cfc8 Mon Sep 17 00:00:00 2001 From: Brian Conry Date: Wed, 23 Feb 2022 14:16:44 -0600 Subject: [PATCH 1/3] Set X-Content-Type-Options to nosniff to tell browser not to change content-type This addresses CVE-2022-25802. When browser encounters a content-type it doesn't recognize, it can "sniff" the content and choose one suitable for the content, which is risky as it could execute the content(e.g. as HTML with js) that is from untrusted external source. Setting X-Content-Type-Options to "nosniff" disables browser's "sniff" behavior. By setting it here, it not only fixes the issue above, but also potentially provides better CORB-protection for various content types. See also https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Content-Type-Options There is a config option to disable this if necessary. --- etc/RT_Config.pm.in | 12 ++++++++++++ share/html/Download/CustomFieldValue/dhandler | 1 + share/html/Ticket/Attachment/dhandler | 1 + 3 files changed, 14 insertions(+) diff --git a/etc/RT_Config.pm.in b/etc/RT_Config.pm.in index 97afc855d53..35d5ffc794c 100644 --- a/etc/RT_Config.pm.in +++ b/etc/RT_Config.pm.in @@ -2574,6 +2574,18 @@ if there are other query arguments. Set( %ReferrerComponents ); +=item C<$StrictContentTypes> + +If set to 0, the C 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 key diff --git a/share/html/Download/CustomFieldValue/dhandler b/share/html/Download/CustomFieldValue/dhandler index 688160bdd9b..c2624957a0e 100644 --- a/share/html/Download/CustomFieldValue/dhandler +++ b/share/html/Download/CustomFieldValue/dhandler @@ -70,6 +70,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 44ada1e211a..1492303ffa7 100644 --- 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); From 8907571045db2297da0ff865570cb7c605c2342f Mon Sep 17 00:00:00 2001 From: sunnavy Date: Tue, 24 May 2022 21:28:54 +0800 Subject: [PATCH 2/3] Add ACL check to ObjectCustomFieldValues --- lib/RT/ObjectCustomFieldValue.pm | 32 ++++++++++++++++++++++++++++--- lib/RT/ObjectCustomFieldValues.pm | 9 +++++++++ lib/RT/Record.pm | 3 ++- lib/RT/System.pm | 3 ++- 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/lib/RT/ObjectCustomFieldValue.pm b/lib/RT/ObjectCustomFieldValue.pm index 1a11e13e13b..db14fd9fde0 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 ce698f5dcd1..9705b2f2d29 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 bf7d8abef80..65d700b44bb 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 0c90f3c88e9..8e7839b4d3b 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); } From 03f12ca042121c94c5f35f736cabd8da84275be2 Mon Sep 17 00:00:00 2001 From: sunnavy Date: Tue, 24 May 2022 21:31:19 +0800 Subject: [PATCH 3/3] Add ACL check to /Download/CustomFieldValue/... endpoints --- share/html/Download/CustomFieldValue/dhandler | 2 ++ 1 file changed, 2 insertions(+) diff --git a/share/html/Download/CustomFieldValue/dhandler b/share/html/Download/CustomFieldValue/dhandler index 688160bdd9b..7825104636f 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')) {