View | Details | Raw Unified | Return to bug 220031 | Differences between
and this patch

Collapse All | Expand All

(-)files/patch-Makefile.in (-1 / +1 lines)
Lines 1-4 Link Here
1
--- Makefile.in.orig	2016-07-18 UTC
1
--- Makefile.in.orig	2016-07-18 20:20:17 UTC
2
+++ Makefile.in
2
+++ Makefile.in
3
@@ -59,7 +59,7 @@ RT_LAYOUT		=	@rt_layout_name@
3
@@ -59,7 +59,7 @@ RT_LAYOUT		=	@rt_layout_name@
4
 
4
 
(-)files/patch-configure (-2 / +2 lines)
Lines 1-6 Link Here
1
--- configure.orig	2014-09-11 19:03:07 UTC
1
--- configure.orig	2016-07-20 15:48:58 UTC
2
+++ configure
2
+++ configure
3
@@ -2088,7 +2088,7 @@
3
@@ -2112,7 +2112,7 @@ $as_echo "$as_me: WARNING: Layout file $
4
 		s/^#.*$//m;
4
 		s/^#.*$//m;
5
 		s/^\s+//gim;
5
 		s/^\s+//gim;
6
 		s/\s+$/\n/gim;
6
 		s/\s+$/\n/gim;
(-)files/patch-lib_RT.pm (+13 lines)
Line 0 Link Here
1
--- lib/RT.pm.orig	2016-07-18 20:20:17 UTC
2
+++ lib/RT.pm
3
@@ -81,6 +81,10 @@ use vars qw($BasePath
4
  $MasonDataDir
5
  $MasonSessionDir);
6
 
7
+# Set Email::Address module var before anything else loads.
8
+# This avoids an algorithmic complexity denial of service vulnerability.
9
+# See T#157608 and CVE-2015-7686 for more information.
10
+$Email::Address::COMMENT_NEST_LEVEL = 1;
11
 
12
 RT->LoadGeneratedData();
13
 
(-)files/patch-lib_RT_Authen_ExternalAuth_DBI.pm (+54 lines)
Line 0 Link Here
1
--- lib/RT/Authen/ExternalAuth/DBI.pm.orig	2016-07-18 20:20:17 UTC
2
+++ lib/RT/Authen/ExternalAuth/DBI.pm
3
@@ -50,6 +50,7 @@ package RT::Authen::ExternalAuth::DBI;
4
 
5
 use DBI;
6
 use RT::Authen::ExternalAuth::DBI::Cookie;
7
+use RT::Util;
8
 
9
 use warnings;
10
 use strict;
11
@@ -81,6 +82,7 @@ Provides the database implementation for
12
             'p_field'                   =>  'password',
13
 
14
             # Example of custom hashed password check
15
+            # (See below for security concerns with this implementation)
16
             #'p_check'                   =>  sub {
17
             #    my ($hash_from_db, $password) = @_;
18
             #    return $hash_from_db eq function($password);
19
@@ -170,6 +172,17 @@ An example, where C<FooBar()> is some ex
20
 Importantly, the C<p_check> subroutine allows for arbitrarily complex password
21
 checking unlike C<p_enc_pkg> and C<p_enc_sub>.
22
 
23
+Please note, the use of the C<eq> operator in the C<p_check> example above
24
+introduces a timing sidechannel vulnerability. (It was left there for clarity
25
+of the example.) There is a comparison function available in RT that is
26
+hardened against timing attacks. The comparison from the above example could
27
+be re-written with it like this:
28
+
29
+    p_check => sub {
30
+        my ($hash_from_db, $password) = @_;
31
+        return RT::Util::constant_time_eq($hash_from_db, FooBar($password));
32
+    },
33
+
34
 =item p_enc_pkg, p_enc_sub
35
 
36
 The Perl package and subroutine used to encrypt passwords from the
37
@@ -298,7 +311,7 @@ sub GetAuth {
38
         # Jump to the next external authentication service if they don't match
39
         if(defined($db_p_salt)) {
40
             $RT::Logger->debug("Using salt:",$db_p_salt);
41
-            if(${encrypt}->($password,$db_p_salt) ne $pass_from_db){
42
+            unless (RT::Util::constant_time_eq(${encrypt}->($password,$db_p_salt), $pass_from_db)) {
43
                 $RT::Logger->info(  $service,
44
                                     "AUTH FAILED",
45
                                     $username,
46
@@ -306,7 +319,7 @@ sub GetAuth {
47
                 return 0;
48
             }
49
         } else {
50
-            if(${encrypt}->($password) ne $pass_from_db){
51
+            unless (RT::Util::constant_time_eq(${encrypt}->($password), $pass_from_db)) {
52
                 $RT::Logger->info(  $service,
53
                                     "AUTH FAILED",
54
                                     $username,
(-)files/patch-lib_RT_Config.pm (+17 lines)
Line 0 Link Here
1
--- lib/RT/Config.pm.orig	2016-07-18 20:20:17 UTC
2
+++ lib/RT/Config.pm
3
@@ -147,6 +147,14 @@ can be set for each config optin:
4
 our %META;
5
 %META = (
6
     # General user overridable options
7
+    RestrictReferrerLogin => {
8
+        PostLoadCheck => sub {
9
+            my $self = shift;
10
+            if (defined($self->Get('RestrictReferrerLogin'))) {
11
+                RT::Logger->error("The config option 'RestrictReferrerLogin' is incorrect, and should be 'RestrictLoginReferrer' instead.");
12
+            }
13
+        },
14
+    },
15
     DefaultQueue => {
16
         Section         => 'General',
17
         Overridable     => 1,
(-)files/patch-lib_RT_Interface_Web.pm (+20 lines)
Line 0 Link Here
1
--- lib/RT/Interface/Web.pm.orig	2016-07-18 20:20:17 UTC
2
+++ lib/RT/Interface/Web.pm
3
@@ -1448,7 +1448,7 @@ sub IsCompCSRFWhitelisted {
4
     # golden.  This acts on the presumption that external forms may
5
     # hardcode a username and password -- if a malicious attacker knew
6
     # both already, CSRF is the least of your problems.
7
-    my $AllowLoginCSRF = not RT->Config->Get('RestrictReferrerLogin');
8
+    my $AllowLoginCSRF = not RT->Config->Get('RestrictLoginReferrer');
9
     if ($AllowLoginCSRF and defined($args{user}) and defined($args{pass})) {
10
         my $user_obj = RT::CurrentUser->new();
11
         $user_obj->Load($args{user});
12
@@ -1666,7 +1666,7 @@ sub MaybeShowInterstitialCSRFPage {
13
     my $token = StoreRequestToken($ARGS);
14
     $HTML::Mason::Commands::m->comp(
15
         '/Elements/CSRF',
16
-        OriginalURL => RT->Config->Get('WebPath') . $HTML::Mason::Commands::r->path_info,
17
+        OriginalURL => RT->Config->Get('WebBaseURL') . RT->Config->Get('WebPath') . $HTML::Mason::Commands::r->path_info,
18
         Reason => HTML::Mason::Commands::loc( $msg, @loc ),
19
         Token => $token,
20
     );
(-)files/patch-lib_RT_User.pm (+87 lines)
Line 0 Link Here
1
--- lib/RT/User.pm.orig	2016-07-18 20:20:17 UTC
2
+++ lib/RT/User.pm
3
@@ -84,6 +84,7 @@ use RT::Principals;
4
 use RT::ACE;
5
 use RT::Interface::Email;
6
 use Text::Password::Pronounceable;
7
+use RT::Util;
8
 
9
 sub _OverlayAccessible {
10
     {
11
@@ -1087,11 +1088,17 @@ sub IsPassword {
12
         # If it's a new-style (>= RT 4.0) password, it starts with a '!'
13
         my (undef, $method, @rest) = split /!/, $stored;
14
         if ($method eq "bcrypt") {
15
-            return 0 unless $self->_GeneratePassword_bcrypt($value, @rest) eq $stored;
16
+            return 0 unless RT::Util::constant_time_eq(
17
+                $self->_GeneratePassword_bcrypt($value, @rest),
18
+                $stored
19
+            );
20
             # Upgrade to a larger number of rounds if necessary
21
             return 1 unless $rest[0] < RT->Config->Get('BcryptCost');
22
         } elsif ($method eq "sha512") {
23
-            return 0 unless $self->_GeneratePassword_sha512($value, @rest) eq $stored;
24
+            return 0 unless RT::Util::constant_time_eq(
25
+                $self->_GeneratePassword_sha512($value, @rest),
26
+                $stored
27
+            );
28
         } else {
29
             $RT::Logger->warn("Unknown hash method $method");
30
             return 0;
31
@@ -1101,16 +1108,28 @@ sub IsPassword {
32
         my $hash = MIME::Base64::decode_base64($stored);
33
         # Decoding yields 30 byes; first 4 are the salt, the rest are substr(SHA256,0,26)
34
         my $salt = substr($hash, 0, 4, "");
35
-        return 0 unless substr(Digest::SHA::sha256($salt . Digest::MD5::md5(Encode::encode( "UTF-8", $value))), 0, 26) eq $hash;
36
+        return 0 unless RT::Util::constant_time_eq(
37
+            substr(Digest::SHA::sha256($salt . Digest::MD5::md5(Encode::encode( "UTF-8", $value))), 0, 26),
38
+            $hash
39
+        );
40
     } elsif (length $stored == 32) {
41
         # Hex nonsalted-md5
42
-        return 0 unless Digest::MD5::md5_hex(Encode::encode( "UTF-8", $value)) eq $stored;
43
+        return 0 unless RT::Util::constant_time_eq(
44
+            Digest::MD5::md5_hex(Encode::encode( "UTF-8", $value)),
45
+            $stored
46
+        );
47
     } elsif (length $stored == 22) {
48
         # Base64 nonsalted-md5
49
-        return 0 unless Digest::MD5::md5_base64(Encode::encode( "UTF-8", $value)) eq $stored;
50
+        return 0 unless RT::Util::constant_time_eq(
51
+            Digest::MD5::md5_base64(Encode::encode( "UTF-8", $value)),
52
+            $stored
53
+        );
54
     } elsif (length $stored == 13) {
55
         # crypt() output
56
-        return 0 unless crypt(Encode::encode( "UTF-8", $value), $stored) eq $stored;
57
+        return 0 unless RT::Util::constant_time_eq(
58
+            crypt(Encode::encode( "UTF-8", $value), $stored),
59
+            $stored
60
+        );
61
     } else {
62
         $RT::Logger->warning("Unknown password form");
63
         return 0;
64
@@ -1206,19 +1225,20 @@ sub GenerateAuthString {
65
 
66
 =head3 ValidateAuthString
67
 
68
-Takes auth string and protected string. Returns true is protected string
69
+Takes auth string and protected string. Returns true if protected string
70
 has been protected by user's L</AuthToken>. See also L</GenerateAuthString>.
71
 
72
 =cut
73
 
74
 sub ValidateAuthString {
75
     my $self = shift;
76
-    my $auth_string = shift;
77
+    my $auth_string_to_validate = shift;
78
     my $protected = shift;
79
 
80
     my $str = Encode::encode( "UTF-8", $self->AuthToken . $protected );
81
+    my $valid_auth_string = substr(Digest::MD5::md5_hex($str),0,16);
82
 
83
-    return $auth_string eq substr(Digest::MD5::md5_hex($str),0,16);
84
+    return RT::Util::constant_time_eq( $auth_string_to_validate, $valid_auth_string );
85
 }
86
 
87
 =head2 SetDisabled
(-)files/patch-lib_RT_Util.pm (+70 lines)
Line 0 Link Here
1
--- lib/RT/Util.pm.orig	2016-07-18 20:20:17 UTC
2
+++ lib/RT/Util.pm
3
@@ -54,6 +54,8 @@ use warnings;
4
 use base 'Exporter';
5
 our @EXPORT = qw/safe_run_child mime_recommended_filename/;
6
 
7
+use Encode qw/encode/;
8
+
9
 sub safe_run_child (&) {
10
     my $our_pid = $$;
11
 
12
@@ -150,6 +152,58 @@ sub assert_bytes {
13
 }
14
 
15
 
16
+=head2 C<constant_time_eq($a, $b)>
17
+
18
+Compares two strings for equality in constant-time. Replacement for the C<eq>
19
+operator designed to avoid timing side-channel vulnerabilities. Returns zero
20
+or one.
21
+
22
+This is intended for use in cryptographic subsystems for comparing well-formed
23
+data such as hashes - not for direct use with user input or as a general
24
+replacement for the C<eq> operator.
25
+
26
+The two string arguments B<MUST> be of equal length. If the lengths differ,
27
+this function will call C<die()>, as proceeding with execution would create
28
+a timing vulnerability. Length is defined by characters, not bytes.
29
+
30
+This code has been tested to do what it claims. Do not change it without
31
+thorough statistical timing analysis to validate the changes.
32
+
33
+Added to resolve CVE-2017-5361
34
+
35
+For more on timing attacks, see this Wikipedia article:
36
+B<https://en.wikipedia.org/wiki/Timing_attack>
37
+
38
+=cut
39
+
40
+sub constant_time_eq {
41
+    my ($a, $b) = @_;
42
+
43
+    my $result = 0;
44
+
45
+    # generic error message avoids potential information leaks
46
+    my $generic_error = "Cannot compare values";
47
+    die $generic_error unless defined $a and defined $b;
48
+    die $generic_error unless length $a == length $b;
49
+    die $generic_error if ref($a) or ref($b);
50
+
51
+    for (my $i = 0; $i < length($a); $i++) {
52
+        my $a_char = substr($a, $i, 1);
53
+        my $b_char = substr($b, $i, 1);
54
+
55
+        # encode() is set to die on malformed
56
+        my @a_octets = unpack("C*", encode('UTF-8', $a_char, Encode::FB_CROAK));
57
+        my @b_octets = unpack("C*", encode('UTF-8', $b_char, Encode::FB_CROAK));
58
+        die $generic_error if (scalar @a_octets) != (scalar @b_octets);
59
+
60
+        for (my $j = 0; $j < scalar @a_octets; $j++) {
61
+            $result |= $a_octets[$j] ^ $b_octets[$j];
62
+        }
63
+    }
64
+    return 0 + not $result;
65
+}
66
+
67
+
68
 RT::Base->_ImportOverlays();
69
 
70
 1;
(-)files/patch-sbin_rt-test-dependencies (+11 lines)
Line 0 Link Here
1
--- sbin/rt-test-dependencies.orig	2016-07-20 15:49:00 UTC
2
+++ sbin/rt-test-dependencies
3
@@ -136,7 +136,7 @@ Devel::StackTrace 1.19
4
 Digest::base
5
 Digest::MD5 2.27
6
 Digest::SHA
7
-Email::Address 1.897
8
+Email::Address 1.908
9
 Email::Address::List 0.02
10
 Encode 2.64
11
 Errno
(-)files/patch-share_html_Dashboards_Subscription.html (+11 lines)
Line 0 Link Here
1
--- share/html/Dashboards/Subscription.html.orig	2016-07-18 20:20:17 UTC
2
+++ share/html/Dashboards/Subscription.html
3
@@ -75,7 +75,7 @@
4
 <ol class="dashboard-queries">
5
 %    for my $portlet (@portlets) {
6
         <li class="dashboard-query">
7
-            <% loc($portlet->{description}, $fields{'Rows'}) %>
8
+            <% loc( RT::SavedSearch->EscapeDescription($portlet->{description}), $fields{'Rows'}) %>
9
         </li>
10
 %    }
11
 </ol>
(-)files/patch-share_html_Ticket_Attachment_dhandler (+18 lines)
Line 0 Link Here
1
--- share/html/Ticket/Attachment/dhandler.orig	2016-07-18 20:20:17 UTC
2
+++ share/html/Ticket/Attachment/dhandler
3
@@ -68,11 +68,13 @@ unless ( $AttachmentObj->TransactionId()
4
 my $content = $AttachmentObj->OriginalContent;
5
 my $content_type = $AttachmentObj->ContentType || 'text/plain';
6
 
7
-if ( RT->Config->Get('AlwaysDownloadAttachments') ) {
8
+my $attachment_regex = qr{^(image/svg\+xml|application/pdf)}i;
9
+if ( RT->Config->Get('AlwaysDownloadAttachments') || ($content_type =~ $attachment_regex) ) {
10
     $r->headers_out->{'Content-Disposition'} = "attachment";
11
 }
12
 elsif ( !RT->Config->Get('TrustHTMLAttachments') ) {
13
-    $content_type = 'text/plain' if ( $content_type =~ /^text\/html/i );
14
+    my $text_plain_regex = qr{^(text/html|application/xhtml\+xml|text/xml|application/xml)}i;
15
+    $content_type = 'text/plain' if ( $content_type =~ $text_plain_regex );
16
 }
17
 elsif (lc $content_type eq 'text/html') {
18
     # If we're trusting and serving HTML for display not download, try to do

Return to bug 220031