Permalink
Browse files

[#709] Fix cache poisoning vector if credential caching is enabled.

The cache did not distinguish between cached credentials for read and write
access. As it does not check permissions again if there is a cache hit, users
with authorization for either reading or writing could poison the cache and
subsequently authorize themselves for both access types.

Original fix is by Jean-Philippe Lang, http://www.redmine.org/issues/9567
  • Loading branch information...
meineerde committed Nov 28, 2011
1 parent 24538a4 commit 5e171001bc3a1f623b0bfa9e3cc5c4f37ccbedf2
Showing with 5 additions and 3 deletions.
  1. +5 −3 extra/svn/Redmine.pm
View
@@ -438,10 +438,12 @@ sub is_member {
my $pass_digest = Digest::SHA1::sha1_hex($redmine_pass);
+ my $access_mode = request_is_read_only($r) ? "R" : "W";
+
my $cfg = Apache2::Module::get_config(__PACKAGE__, $r->server, $r->per_dir_config);
my $usrprojpass;
if ($cfg->{RedmineCacheCredsMax}) {
- $usrprojpass = $cfg->{RedmineCacheCreds}->get($redmine_user.":".$project_id);
+ $usrprojpass = $cfg->{RedmineCacheCreds}->get($redmine_user.":".$project_id.":".$access_mode);
return 1 if (defined $usrprojpass and ($usrprojpass eq $pass_digest));
}
my $query = $cfg->{RedmineQuery};
@@ -485,10 +487,10 @@ sub is_member {
if ($cfg->{RedmineCacheCredsMax} and $ret) {
if (defined $usrprojpass) {
- $cfg->{RedmineCacheCreds}->set($redmine_user.":".$project_id, $pass_digest);
+ $cfg->{RedmineCacheCreds}->set($redmine_user.":".$project_id.":".$access_mode, $pass_digest);
} else {
if ($cfg->{RedmineCacheCredsCount} < $cfg->{RedmineCacheCredsMax}) {
- $cfg->{RedmineCacheCreds}->set($redmine_user.":".$project_id, $pass_digest);
+ $cfg->{RedmineCacheCreds}->set($redmine_user.":".$project_id.":".$access_mode, $pass_digest);
$cfg->{RedmineCacheCredsCount}++;
} else {
$cfg->{RedmineCacheCreds}->clear();

0 comments on commit 5e17100

Please sign in to comment.