[Rt-commit] rt branch, 4.2.10-releng, updated. rt-4.2.10

Alex Vandiver alexmv at bestpractical.com
Thu Feb 26 12:48:25 EST 2015


The branch, 4.2.10-releng has been updated
       via  b7377382a22b4b2be5661461e02f887cbeb1a71a (commit)
       via  4062bebfbd13a87e5757d3571e60d58f24f728a5 (commit)
       via  1310eb5346be1bdf2c169705f904cd1b6d6aab96 (commit)
       via  4d257d9a26ba900ba575b8775b2333728766216d (commit)
       via  7b923f3de4a57673aa3895bf5286aaf13de4a4fa (commit)
       via  dbcb10df651a1c9cfb6700afed91a78289aaf677 (commit)
       via  216d5a0c64e6bafeb9fb611ea1e646a92e50a7b2 (commit)
      from  007058892e2ab92b529bd102648b4b89cbfa49ae (commit)

Summary of changes:
 lib/RT/ACL.pm                             | 33 ++++++++++---------------------
 lib/RT/Articles.pm                        | 28 +++++++-------------------
 lib/RT/Attachments.pm                     | 15 ++++----------
 lib/RT/Classes.pm                         | 27 +++++++------------------
 lib/RT/CustomFields.pm                    | 18 ++++++++---------
 lib/RT/Dashboard.pm                       |  8 ++------
 lib/RT/Groups.pm                          | 29 +++++++++++++++------------
 lib/RT/I18N.pm                            |  5 ++++-
 lib/RT/SavedSearch.pm                     |  3 +--
 lib/RT/Scrips.pm                          | 30 +++++++---------------------
 lib/RT/Template.pm                        | 12 +++++++----
 lib/RT/Templates.pm                       | 32 +++++++-----------------------
 lib/RT/Transactions.pm                    | 23 ++++-----------------
 sbin/rt-test-dependencies.in              |  2 +-
 share/html/Search/Elements/ResultsRSSView |  9 ++++++++-
 15 files changed, 94 insertions(+), 180 deletions(-)

- Log -----------------------------------------------------------------
commit 216d5a0c64e6bafeb9fb611ea1e646a92e50a7b2
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Mon Dec 1 16:58:43 2014 -0500

    Hide utf8 warnings during attempted decoding
    
    EncodeFromToWithCroak is used to exploratorily attempt to decode unknown
    byte strings.  This operation, under Encode::FB_DEFAULT, may generate
    warnings -- lots of warnings.  This can lead to denial of service in
    some situations.  This vulnerability has been assigned CVE-2014-9472.
    
    Unfortunately, "no warnings 'utf8'" does not work to quiet them until
    Encode 2.64, and even then, it only works with encode() and decode(),
    not from_to().  Bump the dependency to 2.64, and switch to
    encode(decode()) instead of from_to().

diff --git a/lib/RT/I18N.pm b/lib/RT/I18N.pm
index a9fc52e..bca4864 100644
--- a/lib/RT/I18N.pm
+++ b/lib/RT/I18N.pm
@@ -728,7 +728,10 @@ sub EncodeFromToWithCroak {
     my $from   = shift;
     my $to     = shift;
 
-    eval { Encode::from_to( $string, $from => $to, Encode::FB_CROAK ); };
+    eval {
+        no warnings 'utf8';
+        $string = Encode::encode( $to, Encode::decode( $from, $string ), Encode::FB_CROAK );
+    };
     return $@ ? ( 0, $@ ) : ( 1, $string );
 }
 
diff --git a/sbin/rt-test-dependencies.in b/sbin/rt-test-dependencies.in
index 0e01553..f8c7188 100644
--- a/sbin/rt-test-dependencies.in
+++ b/sbin/rt-test-dependencies.in
@@ -207,7 +207,7 @@ Digest::MD5 2.27
 Digest::SHA
 Email::Address 1.897
 Email::Address::List 0.02
-Encode 2.39
+Encode 2.64
 Errno
 File::Glob
 File::ShareDir

commit dbcb10df651a1c9cfb6700afed91a78289aaf677
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jan 29 16:10:12 2015 -0500

    Push all ACL'ing into AddRecord, so ItemsArrayRef respects it
    
    Existing ACL machinery was implemented either on AddRecord (which
    applies when data is first pulled from the database), or on Next (which
    applies during iteration).  Applying ACLs in Next, however, means that
    they are skipped if ->ItemsArrayRef is called.
    
    Move all existing ACL checks to AddRecord, formalizing logic into
    ->CurrentUserCanSee when relevant.
    
    This change causes test failures for the Dashboards and SavedSearches;
    the existing behavior relies on showing you all groups you are a member
    of, even if you do not have SeeGroup on those groups.  This behavior
    should not change during a stable series; the following commit will add
    an ACL exception for their use case.

diff --git a/lib/RT/ACL.pm b/lib/RT/ACL.pm
index 4c90f18..89ca69f 100644
--- a/lib/RT/ACL.pm
+++ b/lib/RT/ACL.pm
@@ -188,34 +188,21 @@ sub LimitToPrincipal {
 
 
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    my $ACE = $self->SUPER::Next();
     # Short-circuit having to load up the ->Object
-    return $ACE
-        if $self->CurrentUser->PrincipalObj->Id == RT->SystemUser->Id;
-    if ( ( defined($ACE) ) and ( ref($ACE) ) ) {
-
-        if ( $self->CurrentUser->HasRight( Right  => 'ShowACL',
-                                           Object => $ACE->Object )
-             or $self->CurrentUser->HasRight( Right  => 'ModifyACL',
-                                              Object => $ACE->Object )
-          ) {
-            return ($ACE);
-        }
+    return $self->SUPER::AddRecord( $record )
+        if $record->CurrentUser->PrincipalObj->Id == RT->SystemUser->Id;
 
-        #If the user doesn't have the right to show this ACE
-        else {
-            return ( $self->Next() );
-        }
-    }
-
-    #if there never was any ACE
-    else {
-        return (undef);
-    }
+    my $obj = $record->Object;
+    return unless $self->CurrentUser->HasRight( Right  => 'ShowACL',
+                                                Object => $obj )
+               or $self->CurrentUser->HasRight( Right  => 'ModifyACL',
+                                                Object => $obj );
 
+    return $self->SUPER::AddRecord( $record );
 }
 
 # The singular of ACL is ACE.
diff --git a/lib/RT/Articles.pm b/lib/RT/Articles.pm
index f604acc..4b85eba 100644
--- a/lib/RT/Articles.pm
+++ b/lib/RT/Articles.pm
@@ -64,33 +64,19 @@ sub _Init {
     return $self->SUPER::_Init( @_ );
 }
 
-=head2 Next
+=head2 AddRecord
 
-Returns the next article that this user can see.
+Overrides the collection to ensure that only Articles the user can see
+are returned.
 
 =cut
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    my $Object = $self->SUPER::Next();
-    if ( ( defined($Object) ) and ( ref($Object) ) ) {
-
-        if ( $Object->CurrentUserHasRight('ShowArticle') ) {
-            return ($Object);
-        }
-
-        #If the user doesn't have the right to show this Object
-        else {
-            return ( $self->Next() );
-        }
-    }
-
-    #if there never was any queue
-    else {
-        return (undef);
-    }
-
+    return unless $record->CurrentUserHasRight('ShowArticle');
+    return $self->SUPER::AddRecord( $record );
 }
 
 =head2 Limit { FIELD  => undef, OPERATOR => '=', VALUE => 'undef'} 
diff --git a/lib/RT/Attachments.pm b/lib/RT/Attachments.pm
index 0a3bad1..13cf5cf 100644
--- a/lib/RT/Attachments.pm
+++ b/lib/RT/Attachments.pm
@@ -215,19 +215,12 @@ sub LimitByTicket {
     return;
 }
 
-# {{{ sub Next
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    my $Attachment = $self->SUPER::Next;
-    return $Attachment unless $Attachment;
-
-    if ( $Attachment->TransactionObj->CurrentUserCanSee ) {
-        return $Attachment;
-    } else {
-        # If the user doesn't have the right to show this ticket
-        return $self->Next;
-    }
+    return unless $record->TransactionObj->CurrentUserCanSee;
+    return $self->SUPER::AddRecord( $record );
 }
 
 RT::Base->_ImportOverlays();
diff --git a/lib/RT/Classes.pm b/lib/RT/Classes.pm
index d9e61ff..bf55d52 100644
--- a/lib/RT/Classes.pm
+++ b/lib/RT/Classes.pm
@@ -64,32 +64,19 @@ sub Table {'Classes'}
     return ($self->SUPER::_Init(@_));
  }
 
-=head2 Next
+=head2 AddRecord
 
-Returns the next Object that this user can see.
+Overrides the collection to ensure that only Classes the user can
+see are returned.
 
 =cut
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-
-    my $Object = $self->SUPER::Next();
-    if ((defined($Object)) and (ref($Object))) {
-   if ( $Object->CurrentUserHasRight('SeeClass') ) {
-        return($Object);
-    }
-
-    #If the user doesn't have the right to show this Object
-    else {
-        return($self->Next());
-    }
-    }
-    #if there never was any Object
-    else {
-    return(undef);
-    }
-
+    return unless $record->CurrentUserHasRight('SeeClass');
+    return $self->SUPER::AddRecord( $record );
 }
 
 sub _SingularClass { "RT::Class" }
diff --git a/lib/RT/CustomFields.pm b/lib/RT/CustomFields.pm
index f4ccd44..a93bfc8 100644
--- a/lib/RT/CustomFields.pm
+++ b/lib/RT/CustomFields.pm
@@ -378,22 +378,20 @@ sub _OCFAlias {
 }
 
 
-=head2 Next
+=head2 AddRecord
 
-Returns the next custom field that this user can see.
+Overrides the collection to ensure that only custom fields the user can
+see are returned; also propagates down the L</ContextObject>.
 
 =cut
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    my $CF = $self->SUPER::Next();
-    return $CF unless $CF;
-
-    $CF->SetContextObject( $self->ContextObject );
-
-    return $self->Next unless $CF->CurrentUserHasRight('SeeCustomField');
-    return $CF;
+    $record->SetContextObject( $self->ContextObject );
+    return unless $record->CurrentUserHasRight('SeeCustomField');
+    return $self->SUPER::AddRecord( $record );
 }
 
 =head2 NewItem
diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index cd0aa9f..313cbca 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -459,22 +459,13 @@ sub LimitToDeleted {
 
 
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
     # Don't show groups which the user isn't allowed to see.
-
-    my $Group = $self->SUPER::Next();
-    if ((defined($Group)) and (ref($Group))) {
-        unless ($Group->CurrentUserHasRight('SeeGroup')) {
-            return $self->Next();
-        }
-
-        return $Group;
-    }
-    else {
-        return undef;
-    }
+    return unless $record->CurrentUserHasRight('SeeGroup');
+    return $self->SUPER::AddRecord( $record );
 }
 
 
diff --git a/lib/RT/Scrips.pm b/lib/RT/Scrips.pm
index ef8213c..85f1961 100644
--- a/lib/RT/Scrips.pm
+++ b/lib/RT/Scrips.pm
@@ -238,35 +238,19 @@ sub ApplySortOrder {
     } );
 }
 
-# {{{ sub Next 
+=head2 AddRecord
 
-=head2 Next
-
-Returns the next scrip that this user can see.
+Overrides the collection to ensure that only scrips the user can see are
+returned.
 
 =cut
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-
-    my $Scrip = $self->SUPER::Next();
-    if ((defined($Scrip)) and (ref($Scrip))) {
-
-        if ($Scrip->CurrentUserHasRight('ShowScrips')) {
-            return($Scrip);
-        }
-
-        #If the user doesn't have the right to show this scrip
-        else {
-            return($self->Next());
-        }
-    }
-    #if there never was any scrip
-    else {
-        return(undef);
-    }
-
+    return unless $record->CurrentUserHasRight('ShowScrips');
+    return $self->SUPER::AddRecord( $record );
 }
 
 =head2 Apply
diff --git a/lib/RT/Template.pm b/lib/RT/Template.pm
index 23f150b..d299e0b 100644
--- a/lib/RT/Template.pm
+++ b/lib/RT/Template.pm
@@ -842,10 +842,14 @@ sub CompileCheck {
 sub CurrentUserCanRead {
     my $self =shift;
 
-    return 1 if $self->CurrentUserHasQueueRight('ShowTemplate');
-
-    return $self->CurrentUser->HasRight( Right =>'ShowGlobalTemplates', Object => $RT::System )
-        if !$self->QueueObj->Id;
+    if ($self->__Value('Queue')) {
+        my $queue = RT::Queue->new( RT->SystemUser );
+        $queue->Load( $self->__Value('Queue'));
+        return 1 if $self->CurrentUser->HasRight( Right => 'ShowTemplate', Object => $queue );
+    } else {
+        return 1 if $self->CurrentUser->HasRight( Right => 'ShowGlobalTemplates', Object => $RT::System );
+        return 1 if $self->CurrentUser->HasRight( Right => 'ShowTemplate',        Object => $RT::System );
+    }
 
     return;
 }
diff --git a/lib/RT/Templates.pm b/lib/RT/Templates.pm
index ef38161..93ed4fc 100644
--- a/lib/RT/Templates.pm
+++ b/lib/RT/Templates.pm
@@ -125,37 +125,19 @@ sub LimitToQueue {
 }
 
 
-=head2 Next
+=head2 AddRecord
 
-Returns the next template that this user can see.
+Overrides the collection to ensure that only templates the user can see
+are returned.
 
 =cut
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-
-    my $templ = $self->SUPER::Next();
-    if ((defined($templ)) and (ref($templ))) {
-
-        # If it's part of a queue, and the user can read templates in
-        # that queue, or the user can globally read templates, show it
-        if ($templ->Queue && $templ->CurrentUserHasQueueRight('ShowTemplate') or
-            $templ->CurrentUser->HasRight(Object => $RT::System, Right => 'ShowTemplate') or
-            $templ->CurrentUser->HasRight(Object => $RT::System, Right => 'ShowGlobalTemplates')) {
-            return($templ);
-        }
-
-        #If the user doesn't have the right to show this template
-        else {
-            return($self->Next());
-        }
-    }
-    #if there never was any template
-    else {
-        return(undef);
-    }
-
+    return unless $record->CurrentUserCanRead;
+    return $self->SUPER::AddRecord( $record );
 }
 
 RT::Base->_ImportOverlays();
diff --git a/lib/RT/Transactions.pm b/lib/RT/Transactions.pm
index 4d00e2f..6794e52 100644
--- a/lib/RT/Transactions.pm
+++ b/lib/RT/Transactions.pm
@@ -130,27 +130,12 @@ sub LimitToTicket {
 }
 
 
-sub Next {
+sub AddRecord {
     my $self = shift;
+    my ($record) = @_;
 
-    my $Transaction = $self->SUPER::Next();
-    if ((defined($Transaction)) and (ref($Transaction))) {
-        # If the user can see the transaction's type, then they can
-        #  see the transaction and we should hand it back.
-        if ($Transaction->Type) {
-            return($Transaction);
-        }
-
-        #If the user doesn't have the right to show this ticket
-        else {
-            return($self->Next());
-        }
-    }
-
-    #if there never was any ticket
-    else {
-        return(undef);
-    }
+    return unless $record->CurrentUserCanSee;
+    return $self->SUPER::AddRecord($record);
 }
 
 RT::Base->_ImportOverlays();

commit 7b923f3de4a57673aa3895bf5286aaf13de4a4fa
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Jan 29 16:16:25 2015 -0500

    Allow an exception for ACLs on groups, for the current user's groups
    
    As referenced in the previous commit, Dashboards and Saved Searches
    currently walk around the existing ShowGroup ACL, by dint of having used
    ->ItemsArrayRef.  As the previous commit closes that hole, an explicit
    exception is instead made for group searches which contain the current
    user; this allows the current behavior to be preserved.

diff --git a/lib/RT/Dashboard.pm b/lib/RT/Dashboard.pm
index 7e80461..6d9eeb6 100644
--- a/lib/RT/Dashboard.pm
+++ b/lib/RT/Dashboard.pm
@@ -255,8 +255,7 @@ sub _PrivacyObjects {
 
     my $groups = RT::Groups->new($CurrentUser);
     $groups->LimitToUserDefinedGroups;
-    $groups->WithMember( PrincipalId => $CurrentUser->Id,
-                         Recursively => 1 );
+    $groups->WithCurrentUser;
     push @objects, @{ $groups->ItemsArrayRef };
 
     push @objects, RT::System->new($CurrentUser);
@@ -386,10 +385,7 @@ sub ObjectsForLoading {
         Right             => 'SeeGroupDashboard',
         IncludeSuperusers => $args{IncludeSuperuserGroups},
     );
-    $groups->WithMember(
-        Recursively => 1,
-        PrincipalId => $CurrentUser->UserObj->PrincipalId
-    );
+    $groups->WithCurrentUser;
     my $attrs = $groups->Join(
         ALIAS1 => 'main',
         FIELD1 => 'id',
diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index 313cbca..4d06528 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -272,6 +272,15 @@ sub WithMember {
     return $members;
 }
 
+sub WithCurrentUser {
+    my $self = shift;
+    $self->{with_current_user} = 1;
+    return $self->WithMember(
+        PrincipalId => $self->CurrentUser->PrincipalId,
+        Recursively => 1,
+    );
+}
+
 sub WithoutMember {
     my $self = shift;
     my %args = (
@@ -463,8 +472,11 @@ sub AddRecord {
     my $self = shift;
     my ($record) = @_;
 
-    # Don't show groups which the user isn't allowed to see.
-    return unless $record->CurrentUserHasRight('SeeGroup');
+    # If we've explicitly limited to groups the user is a member of (for
+    # dashboard or savedsearch privacy objects), skip the ACL.
+    return unless $self->{with_current_user}
+        or $record->CurrentUserHasRight('SeeGroup');
+
     return $self->SUPER::AddRecord( $record );
 }
 
diff --git a/lib/RT/SavedSearch.pm b/lib/RT/SavedSearch.pm
index 4fe6323..4dd869b 100644
--- a/lib/RT/SavedSearch.pm
+++ b/lib/RT/SavedSearch.pm
@@ -162,8 +162,7 @@ sub _PrivacyObjects {
 
     my $groups = RT::Groups->new($CurrentUser);
     $groups->LimitToUserDefinedGroups;
-    $groups->WithMember( PrincipalId => $CurrentUser->Id,
-                         Recursively => 1 );
+    $groups->WithCurrentUser;
     if ($has_attr) {
         my $attrs = $groups->Join(
             ALIAS1 => 'main',

commit 4d257d9a26ba900ba575b8775b2333728766216d
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Fri Jan 30 15:03:16 2015 -0500

    Prevent text content from being interpreted as HTML by RSS clients
    
    The ->Content method is used to obtain the data to use in the RSS
    <description> tag.  However, most RSS feed readers display the contents
    of the <description> tag using a HTML rendering engine; this allows
    textual content to be mistakenly rendered as HTML.  This specifically
    includes links, which RSS readers may not hide the "Referer" header of,
    exposing the RSS feed URL and thus allowing for information disclosure.
    This vulnerability has been assigned CVE-2015-1165.
    
    Escape the textual content so that it is not interpreted as HTML by RSS
    readers.  This is suprior to requesting ->Content( Type => "text/html" )
    because it is guaranteed to not contain links, and thus not suffer from
    the above Referer disclosure.

diff --git a/share/html/Search/Elements/ResultsRSSView b/share/html/Search/Elements/ResultsRSSView
index 9190bc8..f392369 100644
--- a/share/html/Search/Elements/ResultsRSSView
+++ b/share/html/Search/Elements/ResultsRSSView
@@ -92,10 +92,17 @@ $rss->channel(
 while ( my $Ticket = $Tickets->Next()) {
     my $creator_str = $Ticket->CreatorObj->Format;
     $creator_str =~ s/[\r\n]//g;
+
+    # Get the plain-text content; it is interpreted as HTML by RSS
+    # readers, so it must be escaped (and is escaped _again_ when
+    # inserted into the XML).
+    my $content = $Ticket->Transactions->First->Content;
+    $content = $m->interp->apply_escapes( $content, 'h');
+
     $rss->add_item(
         title       => $Ticket->Subject || loc('No Subject'),
         link        => $url . "Ticket/Display.html?id=".$Ticket->id,
-        description => $Ticket->Transactions->First->Content,
+        description => $content,
         dc          => {
             creator => $creator_str,
             date    => $Ticket->CreatedObj->W3CDTF,

commit 1310eb5346be1bdf2c169705f904cd1b6d6aab96
Merge: 0070588 4d257d9
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Feb 25 16:30:00 2015 -0500

    Merge branch 'security/4.2/rss-content' into security/4.2.10-releng


commit 4062bebfbd13a87e5757d3571e60d58f24f728a5
Merge: 1310eb5 216d5a0
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Feb 25 16:30:12 2015 -0500

    Merge branch 'security/4.2/decode-warnings' into security/4.2.10-releng


commit b7377382a22b4b2be5661461e02f887cbeb1a71a
Merge: 4062beb 7b923f3
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Wed Feb 25 16:30:21 2015 -0500

    Merge branch 'security/4.2/acl-addrecord' into security/4.2.10-releng


-----------------------------------------------------------------------


More information about the rt-commit mailing list