From cc03b490af32328d1d3d7e805383ace89420f456 Mon Sep 17 00:00:00 2001 From: michel Date: Fri, 20 Mar 2020 19:41:10 +0100 Subject: [PATCH 1/6] Bug fix: use the right CurrentUserCanSetOwner return value. CurrentUserCanSetOwner returns ($ok, $msg), not a boolean. --- share/html/Elements/Tabs | 3 ++- share/html/Ticket/Elements/ShowSummary | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs index 45f0fbee7e2..a6a70e35c2e 100644 --- a/share/html/Elements/Tabs +++ b/share/html/Elements/Tabs @@ -637,7 +637,8 @@ my $build_main_nav = sub { $tabs->child( history => title => loc('History'), path => "/Ticket/History.html?id=" . $id ); my %can = %{ $obj->CurrentUser->PrincipalObj->HasRights( Object => $obj ) }; - $can{'_ModifyOwner'} = $obj->CurrentUserCanSetOwner(); + # since CurrentUserCanSetOwner returns ($ok, $msg), the parens ensure that $can{} gets $ok + ( $can{'_ModifyOwner'} ) = $obj->CurrentUserCanSetOwner(); my $can = sub { unless ($_[0] eq 'ExecuteCode') { return $can{$_[0]} || $can{'SuperUser'}; diff --git a/share/html/Ticket/Elements/ShowSummary b/share/html/Ticket/Elements/ShowSummary index e0b9e913b19..7e8bf6e1acd 100644 --- a/share/html/Ticket/Elements/ShowSummary +++ b/share/html/Ticket/Elements/ShowSummary @@ -108,7 +108,7 @@ $Attachments => undef <%INIT> my $can_modify = $Ticket->CurrentUserHasRight('ModifyTicket'); my $can_modify_cf = $Ticket->CurrentUserHasRight('ModifyCustomField'); -my $can_modify_owner = $Ticket->CurrentUserCanSetOwner(); +my ($can_modify_owner) = $Ticket->CurrentUserCanSetOwner(); my $can_modify_people = $Ticket->CurrentUserHasRight('Watch') || $Ticket->CurrentUserHasRight('WatchAsAdminCc'); From ac92dfecd9e9376faefa1243cdfb0dad6346f526 Mon Sep 17 00:00:00 2001 From: sunnavy Date: Fri, 14 Aug 2020 03:52:19 +0800 Subject: [PATCH 2/6] Find full path for processing acl files on upgrade Starting with perl 5.26, . (the current directory) is no longer in @INC by default, which causes upgrade steps that insert acl to fail when RT::Handle tries to require the file. Set the full path so the acl file can be found. This is to mirror changes in 8c61476b50, which is to fix indexes. --- lib/RT/Handle.pm | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/RT/Handle.pm b/lib/RT/Handle.pm index 379523a84a7..494228c18f3 100644 --- a/lib/RT/Handle.pm +++ b/lib/RT/Handle.pm @@ -467,6 +467,9 @@ sub InsertACL { $path = $base_path; } + # Get the full path since . is no longer in @INC after perl 5.24 + $path = Cwd::abs_path($path); + local *acl; do $path || return (0, "Couldn't load ACLs: " . $@); my @acl = acl($dbh); From c7d1c70fbbe820eea26bf36716a47c9069e6ff26 Mon Sep 17 00:00:00 2001 From: Jim Brandt Date: Fri, 30 Nov 2018 11:44:17 -0500 Subject: [PATCH 3/6] Find full path for processing index files on upgrade Starting with perl 5.26, . (the current directory) is no longer in @INC by default, which causes upgrade steps that insert indexes to fail when RT::Handle tries to require the file. Set the full path so the indexes file can be found. --- lib/RT/Handle.pm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/RT/Handle.pm b/lib/RT/Handle.pm index 379523a84a7..10b81f4fe58 100644 --- a/lib/RT/Handle.pm +++ b/lib/RT/Handle.pm @@ -72,6 +72,7 @@ use strict; use warnings; use File::Spec; +use Cwd; =head1 METHODS @@ -587,6 +588,9 @@ sub InsertIndexes { } } + # Get the full path since . is no longer in @INC after perl 5.24 + $path = Cwd::abs_path($path); + local $@; eval { require $path; 1 } or return (0, "Couldn't execute '$path': " . $@); From 3f4f89ee599134f6b062215fc97b0bc6473fabaf Mon Sep 17 00:00:00 2001 From: Steven Burr Date: Tue, 17 Nov 2020 16:06:00 -0500 Subject: [PATCH 4/6] Convert to abs path before executing initialdata files Passing certain relative paths to InsertData() would fail since it is using eval ( require $file ) directly. The file path is now converted to its absolute path before attempting to require it. --- lib/RT/Handle.pm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/RT/Handle.pm b/lib/RT/Handle.pm index 379523a84a7..c1c1f9fe3b7 100644 --- a/lib/RT/Handle.pm +++ b/lib/RT/Handle.pm @@ -834,6 +834,8 @@ sub InsertData { @Templates, @CustomFields, @Scrips, @Attributes, @Initial, @Final); local $@; + # Get the full path since . is no longer in @INC after perl 5.24 + $datafile = Cwd::abs_path($datafile); $RT::Logger->debug("Going to load '$datafile' data file"); eval { require $datafile } or return (0, "Couldn't load data from '$datafile' for import:\n\nERROR:". $@); From 6b3f75f80b5f7b2b8d59f0259de696e8ebed25d3 Mon Sep 17 00:00:00 2001 From: Jim Brandt Date: Wed, 27 Jan 2021 09:18:59 -0500 Subject: [PATCH 5/6] Remove extra closing div on Login/Logout pages The Footer template adds a closing div, normally for the PageLayout template, so Login/Logout can omit the closing div for the "body" to follow the same pattern. Fixes: I#37049 --- share/html/Elements/Login | 3 ++- share/html/NoAuth/Logout.html | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/share/html/Elements/Login b/share/html/Elements/Login index 61813942880..b53f13a77f8 100644 --- a/share/html/Elements/Login +++ b/share/html/Elements/Login @@ -100,7 +100,8 @@ jQuery(function(){ <& /Elements/LoginHelp &> % $m->callback( %ARGS, CallbackName => 'AfterForm' ); - +% # Omit closing div for login-body because Footer adds a closing div, +% # normally for the PageLayout template <& /Elements/Footer, Menu => 0 &> <%ARGS> $next => '' diff --git a/share/html/NoAuth/Logout.html b/share/html/NoAuth/Logout.html index beee12955a3..c6a6fa2fe95 100644 --- a/share/html/NoAuth/Logout.html +++ b/share/html/NoAuth/Logout.html @@ -61,7 +61,9 @@ % $m->callback( %ARGS ); - + +% # Omit closing div for login-body because Footer adds a closing div, +% # normally for the PageLayout template <& /Elements/Footer, Menu => 0 &> % $m->abort(); From d16f8cf13c2af517ee55a85e7b91a0267477189f Mon Sep 17 00:00:00 2001 From: Dianne Skoll Date: Fri, 15 Jan 2021 09:15:20 -0500 Subject: [PATCH 6/6] Always check password to avoid timing side channel attacks on login page This addresses CVE-2021-38562. --- lib/RT/Interface/Web.pm | 8 ++++++++ lib/RT/User.pm | 9 ++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm index 4b9daa53114..1d59a2f2355 100644 --- a/lib/RT/Interface/Web.pm +++ b/lib/RT/Interface/Web.pm @@ -804,9 +804,17 @@ sub AttemptPasswordAuthentication { my $user_obj = RT::CurrentUser->new(); $user_obj->Load( $ARGS->{user} ); + # Load the RT system user as well to avoid timing side channel + my $system_user = RT::CurrentUser->new(); + $system_user->Load(1); # User with ID 1 should always exist! + my $m = $HTML::Mason::Commands::m; unless ( $user_obj->id && $user_obj->IsPassword( $ARGS->{pass} ) ) { + if (!$user_obj->id) { + # Avoid timing side channel... always run IsPassword + $system_user->IsPassword( $ARGS->{pass} ); + } $RT::Logger->error("FAILED LOGIN for @{[$ARGS->{user}]} from $ENV{'REMOTE_ADDR'}"); $m->callback( %$ARGS, CallbackName => 'FailedLogin', CallbackPage => '/autohandler' ); return (0, HTML::Mason::Commands::loc('Your username or password is incorrect')); diff --git a/lib/RT/User.pm b/lib/RT/User.pm index 98bb783665d..c5d82df8a54 100644 --- a/lib/RT/User.pm +++ b/lib/RT/User.pm @@ -976,15 +976,18 @@ sub IsPassword { } if ( $self->PrincipalObj->Disabled ) { + # Run the bcrypt generator to avoid timing side-channel attacks + RT::Util::constant_time_eq($self->_GeneratePassword_bcrypt($value), '0' x 64); $RT::Logger->info( "Disabled user " . $self->Name . " tried to log in" ); return (undef); } unless ($self->HasPassword) { - return(undef); - } - + # Run the bcrypt generator to avoid timing side-channel attacks + RT::Util::constant_time_eq($self->_GeneratePassword_bcrypt($value), '0' x 64); + return undef; + } my $stored = $self->__Value('Password'); if ($stored =~ /^!/) { # If it's a new-style (>= RT 4.0) password, it starts with a '!'