rt-4.4.1.patch

   1 diff --git a/lib/RT.pm b/lib/RT.pm
   2 index ccf3c54..80d2b61 100644
   3 --- a/lib/RT.pm
   4 +++ b/lib/RT.pm
   5 @@ -81,6 +81,10 @@ use vars qw($BasePath
   6   $MasonDataDir
   7   $MasonSessionDir);
   8
   9 +# Set Email::Address module var before anything else loads.
  10 +# This avoids an algorithmic complexity denial of service vulnerability.
  11 +# See T#157608 and CVE-2015-7686 for more information.
  12 +$Email::Address::COMMENT_NEST_LEVEL = 1;
  13
  14  RT->LoadGeneratedData();
  15
  16 diff --git a/lib/RT/Authen/ExternalAuth/DBI.pm b/lib/RT/Authen/ExternalAuth/DBI.pm
  17 index 42a157f..4c7f0f3 100644
  18 --- a/lib/RT/Authen/ExternalAuth/DBI.pm
  19 +++ b/lib/RT/Authen/ExternalAuth/DBI.pm
  20 @@ -50,6 +50,7 @@ package RT::Authen::ExternalAuth::DBI;
  21
  22  use DBI;
  23  use RT::Authen::ExternalAuth::DBI::Cookie;
  24 +use RT::Util;
  25
  26  use warnings;
  27  use strict;
  28 @@ -81,6 +82,7 @@ Provides the database implementation for L<RT::Authen::ExternalAuth>.
  29              'p_field'                   =>  'password',
  30
  31              # Example of custom hashed password check
  32 +            # (See below for security concerns with this implementation)
  33              #'p_check'                   =>  sub {
  34              #    my ($hash_from_db, $password) = @_;
  35              #    return $hash_from_db eq function($password);
  36 @@ -170,6 +172,17 @@ An example, where C<FooBar()> is some external hashing function:
  37  Importantly, the C<p_check> subroutine allows for arbitrarily complex password
  38  checking unlike C<p_enc_pkg> and C<p_enc_sub>.
  39
  40 +Please note, the use of the C<eq> operator in the C<p_check> example above
  41 +introduces a timing sidechannel vulnerability. (It was left there for clarity
  42 +of the example.) There is a comparison function available in RT that is
  43 +hardened against timing attacks. The comparison from the above example could
  44 +be re-written with it like this:
  45 +
  46 +    p_check => sub {
  47 +        my ($hash_from_db, $password) = @_;
  48 +        return RT::Util::constant_time_eq($hash_from_db, FooBar($password));
  49 +    },
  50 +
  51  =item p_enc_pkg, p_enc_sub
  52
  53  The Perl package and subroutine used to encrypt passwords from the
  54 @@ -298,7 +311,7 @@ sub GetAuth {
  55          # Jump to the next external authentication service if they don't match
  56          if(defined($db_p_salt)) {
  57              $RT::Logger->debug("Using salt:",$db_p_salt);
  58 -            if(${encrypt}->($password,$db_p_salt) ne $pass_from_db){
  59 +            unless (RT::Util::constant_time_eq(${encrypt}->($password,$db_p_salt), $pass_from_db)) {
  60                  $RT::Logger->info(  $service,
  61                                      "AUTH FAILED",
  62                                      $username,
  63 @@ -306,7 +319,7 @@ sub GetAuth {
  64                  return 0;
  65              }
  66          } else {
  67 -            if(${encrypt}->($password) ne $pass_from_db){
  68 +            unless (RT::Util::constant_time_eq(${encrypt}->($password), $pass_from_db)) {
  69                  $RT::Logger->info(  $service,
  70                                      "AUTH FAILED",
  71                                      $username,
  72 diff --git a/lib/RT/Config.pm b/lib/RT/Config.pm
  73 index 70df38f..81a95f7 100644
  74 --- a/lib/RT/Config.pm
  75 +++ b/lib/RT/Config.pm
  76 @@ -147,6 +147,14 @@ can be set for each config optin:
  77  our %META;
  78  %META = (
  79      # General user overridable options
  80 +    RestrictReferrerLogin => {
  81 +        PostLoadCheck => sub {
  82 +            my $self = shift;
  83 +            if (defined($self->Get('RestrictReferrerLogin'))) {
  84 +                RT::Logger->error("The config option 'RestrictReferrerLogin' is incorrect, and should be 'RestrictLoginReferrer' instead.");
  85 +            }
  86 +        },
  87 +    },
  88      DefaultQueue => {
  89          Section         => 'General',
  90          Overridable     => 1,
  91 diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
  92 index e3cf905..7f9cf6f 100644
  93 --- a/lib/RT/Interface/Web.pm
  94 +++ b/lib/RT/Interface/Web.pm
  95 @@ -1448,7 +1448,7 @@ sub IsCompCSRFWhitelisted {
  96      # golden.  This acts on the presumption that external forms may
  97      # hardcode a username and password -- if a malicious attacker knew
  98      # both already, CSRF is the least of your problems.
  99 -    my $AllowLoginCSRF = not RT->Config->Get('RestrictReferrerLogin');
 100 +    my $AllowLoginCSRF = not RT->Config->Get('RestrictLoginReferrer');
 101      if ($AllowLoginCSRF and defined($args{user}) and defined($args{pass})) {
 102          my $user_obj = RT::CurrentUser->new();
 103          $user_obj->Load($args{user});
 104 @@ -1666,7 +1666,7 @@ sub MaybeShowInterstitialCSRFPage {
 105      my $token = StoreRequestToken($ARGS);
 106      $HTML::Mason::Commands::m->comp(
 107          '/Elements/CSRF',
 108 -        OriginalURL => RT->Config->Get('WebPath') . $HTML::Mason::Commands::r->path_info,
 109 +        OriginalURL => RT->Config->Get('WebBaseURL') . RT->Config->Get('WebPath') . $HTML::Mason::Commands::r->path_info,
 110          Reason => HTML::Mason::Commands::loc( $msg, @loc ),
 111          Token => $token,
 112      );
 113 diff --git a/lib/RT/User.pm b/lib/RT/User.pm
 114 index e9ccca8..0e86d44 100644
 115 --- a/lib/RT/User.pm
 116 +++ b/lib/RT/User.pm
 117 @@ -84,6 +84,7 @@ use RT::Principals;
 118  use RT::ACE;
 119  use RT::Interface::Email;
 120  use Text::Password::Pronounceable;
 121 +use RT::Util;
 122
 123  sub _OverlayAccessible {
 124      {
 125 @@ -1087,11 +1088,17 @@ sub IsPassword {
 126          # If it's a new-style (>= RT 4.0) password, it starts with a '!'
 127          my (undef, $method, @rest) = split /!/, $stored;
 128          if ($method eq "bcrypt") {
 129 -            return 0 unless $self->_GeneratePassword_bcrypt($value, @rest) eq $stored;
 130 +            return 0 unless RT::Util::constant_time_eq(
 131 +                $self->_GeneratePassword_bcrypt($value, @rest),
 132 +                $stored
 133 +            );
 134              # Upgrade to a larger number of rounds if necessary
 135              return 1 unless $rest[0] < RT->Config->Get('BcryptCost');
 136          } elsif ($method eq "sha512") {
 137 -            return 0 unless $self->_GeneratePassword_sha512($value, @rest) eq $stored;
 138 +            return 0 unless RT::Util::constant_time_eq(
 139 +                $self->_GeneratePassword_sha512($value, @rest),
 140 +                $stored
 141 +            );
 142          } else {
 143              $RT::Logger->warn("Unknown hash method $method");
 144              return 0;
 145 @@ -1101,16 +1108,28 @@ sub IsPassword {
 146          my $hash = MIME::Base64::decode_base64($stored);
 147          # Decoding yields 30 byes; first 4 are the salt, the rest are substr(SHA256,0,26)
 148          my $salt = substr($hash, 0, 4, "");
 149 -        return 0 unless substr(Digest::SHA::sha256($salt . Digest::MD5::md5(Encode::encode( "UTF-8", $value))), 0, 26) eq $hash;
 150 +        return 0 unless RT::Util::constant_time_eq(
 151 +            substr(Digest::SHA::sha256($salt . Digest::MD5::md5(Encode::encode( "UTF-8", $value))), 0, 26),
 152 +            $hash
 153 +        );
 154      } elsif (length $stored == 32) {
 155          # Hex nonsalted-md5
 156 -        return 0 unless Digest::MD5::md5_hex(Encode::encode( "UTF-8", $value)) eq $stored;
 157 +        return 0 unless RT::Util::constant_time_eq(
 158 +            Digest::MD5::md5_hex(Encode::encode( "UTF-8", $value)),
 159 +            $stored
 160 +        );
 161      } elsif (length $stored == 22) {
 162          # Base64 nonsalted-md5
 163 -        return 0 unless Digest::MD5::md5_base64(Encode::encode( "UTF-8", $value)) eq $stored;
 164 +        return 0 unless RT::Util::constant_time_eq(
 165 +            Digest::MD5::md5_base64(Encode::encode( "UTF-8", $value)),
 166 +            $stored
 167 +        );
 168      } elsif (length $stored == 13) {
 169          # crypt() output
 170 -        return 0 unless crypt(Encode::encode( "UTF-8", $value), $stored) eq $stored;
 171 +        return 0 unless RT::Util::constant_time_eq(
 172 +            crypt(Encode::encode( "UTF-8", $value), $stored),
 173 +            $stored
 174 +        );
 175      } else {
 176          $RT::Logger->warning("Unknown password form");
 177          return 0;
 178 @@ -1206,19 +1225,20 @@ sub GenerateAuthString {
 179
 180  =head3 ValidateAuthString
 181
 182 -Takes auth string and protected string. Returns true is protected string
 183 +Takes auth string and protected string. Returns true if protected string
 184  has been protected by user's L</AuthToken>. See also L</GenerateAuthString>.
 185
 186  =cut
 187
 188  sub ValidateAuthString {
 189      my $self = shift;
 190 -    my $auth_string = shift;
 191 +    my $auth_string_to_validate = shift;
 192      my $protected = shift;
 193
 194      my $str = Encode::encode( "UTF-8", $self->AuthToken . $protected );
 195 +    my $valid_auth_string = substr(Digest::MD5::md5_hex($str),0,16);
 196
 197 -    return $auth_string eq substr(Digest::MD5::md5_hex($str),0,16);
 198 +    return RT::Util::constant_time_eq( $auth_string_to_validate, $valid_auth_string );
 199  }
 200
 201  =head2 SetDisabled
 202 diff --git a/lib/RT/Util.pm b/lib/RT/Util.pm
 203 index 70b557b..47b1dd2 100644
 204 --- a/lib/RT/Util.pm
 205 +++ b/lib/RT/Util.pm
 206 @@ -54,6 +54,8 @@ use warnings;
 207  use base 'Exporter';
 208  our @EXPORT = qw/safe_run_child mime_recommended_filename/;
 209
 210 +use Encode qw/encode/;
 211 +
 212  sub safe_run_child (&) {
 213      my $our_pid = $$;
 214
 215 @@ -150,6 +152,58 @@ sub assert_bytes {
 216  }
 217
 218
 219 +=head2 C<constant_time_eq($a, $b)>
 220 +
 221 +Compares two strings for equality in constant-time. Replacement for the C<eq>
 222 +operator designed to avoid timing side-channel vulnerabilities. Returns zero
 223 +or one.
 224 +
 225 +This is intended for use in cryptographic subsystems for comparing well-formed
 226 +data such as hashes - not for direct use with user input or as a general
 227 +replacement for the C<eq> operator.
 228 +
 229 +The two string arguments B<MUST> be of equal length. If the lengths differ,
 230 +this function will call C<die()>, as proceeding with execution would create
 231 +a timing vulnerability. Length is defined by characters, not bytes.
 232 +
 233 +This code has been tested to do what it claims. Do not change it without
 234 +thorough statistical timing analysis to validate the changes.
 235 +
 236 +Added to resolve CVE-2017-5361
 237 +
 238 +For more on timing attacks, see this Wikipedia article:
 239 +B<https://en.wikipedia.org/wiki/Timing_attack>
 240 +
 241 +=cut
 242 +
 243 +sub constant_time_eq {
 244 +    my ($a, $b) = @_;
 245 +
 246 +    my $result = 0;
 247 +
 248 +    # generic error message avoids potential information leaks
 249 +    my $generic_error = "Cannot compare values";
 250 +    die $generic_error unless defined $a and defined $b;
 251 +    die $generic_error unless length $a == length $b;
 252 +    die $generic_error if ref($a) or ref($b);
 253 +
 254 +    for (my $i = 0; $i < length($a); $i++) {
 255 +        my $a_char = substr($a, $i, 1);
 256 +        my $b_char = substr($b, $i, 1);
 257 +
 258 +        # encode() is set to die on malformed
 259 +        my @a_octets = unpack("C*", encode('UTF-8', $a_char, Encode::FB_CROAK));
 260 +        my @b_octets = unpack("C*", encode('UTF-8', $b_char, Encode::FB_CROAK));
 261 +        die $generic_error if (scalar @a_octets) != (scalar @b_octets);
 262 +
 263 +        for (my $j = 0; $j < scalar @a_octets; $j++) {
 264 +            $result |= $a_octets[$j] ^ $b_octets[$j];
 265 +        }
 266 +    }
 267 +    return 0 + not $result;
 268 +}
 269 +
 270 +
 271  RT::Base->_ImportOverlays();
 272
 273  1;
 274 diff --git a/sbin/rt-test-dependencies b/sbin/rt-test-dependencies
 275 index 0e57ca1..07bb082 100644
 276 --- a/sbin/rt-test-dependencies
 277 +++ b/sbin/rt-test-dependencies
 278 @@ -136,7 +136,7 @@ Devel::StackTrace 1.19
 279  Digest::base
 280  Digest::MD5 2.27
 281  Digest::SHA
 282 -Email::Address 1.897
 283 +Email::Address 1.908
 284  Email::Address::List 0.02
 285  Encode 2.64
 286  Errno
 287 diff --git a/share/html/Dashboards/Subscription.html b/share/html/Dashboards/Subscription.html
 288 index 34aaa33..36abd2a 100644
 289 --- a/share/html/Dashboards/Subscription.html
 290 +++ b/share/html/Dashboards/Subscription.html
 291 @@ -75,7 +75,7 @@
 292  <ol class="dashboard-queries">
 293  %    for my $portlet (@portlets) {
 294          <li class="dashboard-query">
 295 -            <% loc($portlet->{description}, $fields{'Rows'}) %>
 296 +            <% loc( RT::SavedSearch->EscapeDescription($portlet->{description}), $fields{'Rows'}) %>
 297          </li>
 298  %    }
 299  </ol>
 300 diff --git a/share/html/Ticket/Attachment/dhandler b/share/html/Ticket/Attachment/dhandler
 301 index 3d7c07b..c6ca376 100644
 302 --- a/share/html/Ticket/Attachment/dhandler
 303 +++ b/share/html/Ticket/Attachment/dhandler
 304 @@ -68,11 +68,13 @@ unless ( $AttachmentObj->TransactionId() == $trans ) {
 305  my $content = $AttachmentObj->OriginalContent;
 306  my $content_type = $AttachmentObj->ContentType || 'text/plain';
 307
 308 -if ( RT->Config->Get('AlwaysDownloadAttachments') ) {
 309 +my $attachment_regex = qr{^(image/svg\+xml|application/pdf)}i;
 310 +if ( RT->Config->Get('AlwaysDownloadAttachments') || ($content_type =~ $attachment_regex) ) {
 311      $r->headers_out->{'Content-Disposition'} = "attachment";
 312  }
 313  elsif ( !RT->Config->Get('TrustHTMLAttachments') ) {
 314 -    $content_type = 'text/plain' if ( $content_type =~ /^text\/html/i );
 315 +    my $text_plain_regex = qr{^(text/html|application/xhtml\+xml|text/xml|application/xml)}i;
 316 +    $content_type = 'text/plain' if ( $content_type =~ $text_plain_regex );
 317  }
 318  elsif (lc $content_type eq 'text/html') {
 319      # If we're trusting and serving HTML for display not download, try to do