diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index b987b0977..f37dca274 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -17,6 +17,7 @@ - fix: Add missing page on Problem With Request - fix: Avatar/photo upload not working - fix: Shortcut with content lock fails (Thanks to Michelle Lamar) + - fix: Security bug in session env 7.0.0 diff --git a/lib/WebGUI/Asset/Post.pm b/lib/WebGUI/Asset/Post.pm index 86700304f..d95a32d5e 100644 --- a/lib/WebGUI/Asset/Post.pm +++ b/lib/WebGUI/Asset/Post.pm @@ -532,7 +532,7 @@ sub hasRated { return 1 if $self->isPoster; my $flag = 0; if ($self->session->user->userId eq "1") { - ($flag) = $self->session->db->quickArray("select count(*) from Post_rating where assetId=? and ipAddress=?",[$self->getId, $self->session->env->get("REMOTE_ADDR")]); + ($flag) = $self->session->db->quickArray("select count(*) from Post_rating where assetId=? and ipAddress=?",[$self->getId, $self->session->env->getIp]); } else { ($flag) = $self->session->db->quickArray("select count(*) from Post_rating where assetId=? and userId=?",[$self->getId, $self->session->user->userId]); } @@ -776,7 +776,7 @@ sub rate { return undef unless ($rating == -1 || $rating == 1); unless ($self->hasRated) { $self->session->db->write("insert into Post_rating (assetId,userId,ipAddress,dateOfRating,rating) values (" - .$self->session->db->quote($self->getId).", ".$self->session->db->quote($self->session->user->userId).", ".$self->session->db->quote($self->session->env->get("REMOTE_ADDR")).", + .$self->session->db->quote($self->getId).", ".$self->session->db->quote($self->session->user->userId).", ".$self->session->db->quote($self->session->env->getIp).", ".$self->session->datetime->time().", ".$self->session->db->quote($rating).")"); my ($sum) = $self->session->db->quickArray("select sum(rating) from Post_rating where assetId=".$self->session->db->quote($self->getId)); $self->update({rating=>$sum}); diff --git a/lib/WebGUI/Asset/Post/Thread.pm b/lib/WebGUI/Asset/Post/Thread.pm index 167d8b76c..57057af92 100644 --- a/lib/WebGUI/Asset/Post/Thread.pm +++ b/lib/WebGUI/Asset/Post/Thread.pm @@ -553,7 +553,7 @@ sub rate { return undef unless ($rating == -1 || $rating == 1); unless ($self->hasRated) { $self->session->db->write("insert into Post_rating (assetId,userId,ipAddress,dateOfRating,rating) values (" - .$self->session->db->quote($self->getId).", ".$self->session->db->quote($self->session->user->userId).", ".$self->session->db->quote($self->session->env->get("REMOTE_ADDR")).", + .$self->session->db->quote($self->getId).", ".$self->session->db->quote($self->session->user->userId).", ".$self->session->db->quote($self->session->env->getIp).", ".$self->session->datetime->time().", ".$self->session->db->quote($rating).")"); my ($sum) = $self->session->db->quickArray("select sum(Post.rating) from Post left join asset on Post.assetId=asset.assetId where Post.threadId=".$self->session->db->quote($self->getId)." and Post.rating>0"); $self->update({rating=>$sum}); diff --git a/lib/WebGUI/Asset/Wobject/DataForm.pm b/lib/WebGUI/Asset/Wobject/DataForm.pm index 0e4de9a8e..b93be91e0 100644 --- a/lib/WebGUI/Asset/Wobject/DataForm.pm +++ b/lib/WebGUI/Asset/Wobject/DataForm.pm @@ -1091,7 +1091,7 @@ sub www_process { assetId=>$self->getId, userId=>$self->session->user->userId, username=>$self->session->user->username, - ipAddress=>$self->session->env->get("REMOTE_ADDR"), + ipAddress=>$self->session->env->getIp, submissionDate=>$self->session->datetime->time() },0); my ($var, %row, @errors, $updating, $hadErrors); diff --git a/lib/WebGUI/Asset/Wobject/Poll.pm b/lib/WebGUI/Asset/Wobject/Poll.pm index f8dc92548..f8e566d38 100644 --- a/lib/WebGUI/Asset/Wobject/Poll.pm +++ b/lib/WebGUI/Asset/Wobject/Poll.pm @@ -28,7 +28,7 @@ sub _hasVoted { my $self = shift; my ($hasVoted) = $self->session->db->quickArray("select count(*) from Poll_answer where assetId=".$self->session->db->quote($self->getId)." and ((userId=".$self->session->db->quote($self->session->user->userId)." - and userId<>'1') or (userId=".$self->session->db->quote($self->session->user->userId)." and ipAddress='".$self->session->env->get("REMOTE_ADDR")."'))"); + and userId<>'1') or (userId=".$self->session->db->quote($self->session->user->userId)." and ipAddress='".$self->session->env->getIp."'))"); return $hasVoted; } @@ -412,7 +412,7 @@ sub www_vote { my $self = shift; my $u; if ($self->session->form->process("answer") ne "" && $self->session->user->isInGroup($self->get("voteGroup")) && !($self->_hasVoted())) { - $self->setVote($self->session->form->process("answer"),$self->session->user->userId,$self->session->env->get("REMOTE_ADDR")); + $self->setVote($self->session->form->process("answer"),$self->session->user->userId,$self->session->env->getIp); if ($self->session->setting->get("useKarma")) { $self->session->user->karma($self->get("karmaPerVote"),"Poll (".$self->getId.")","Voted on this poll."); } diff --git a/lib/WebGUI/Asset/Wobject/Survey.pm b/lib/WebGUI/Asset/Wobject/Survey.pm index f84398f78..377ab8273 100644 --- a/lib/WebGUI/Asset/Wobject/Survey.pm +++ b/lib/WebGUI/Asset/Wobject/Survey.pm @@ -322,7 +322,7 @@ sub getEditForm { #------------------------------------------------------------------- sub getIp { my $self = shift; - my $ip = ($self->get("anonymous")) ? substr(md5_hex($self->session->env->get("REMOTE_ADDR")),0,8) : $self->session->env->get("REMOTE_ADDR"); + my $ip = ($self->get("anonymous")) ? substr(md5_hex($self->session->env->getIp),0,8) : $self->session->env->getIp; return $ip; } diff --git a/lib/WebGUI/Auth.pm b/lib/WebGUI/Auth.pm index 26d44d833..ceb8fa08d 100644 --- a/lib/WebGUI/Auth.pm +++ b/lib/WebGUI/Auth.pm @@ -94,7 +94,7 @@ sub _isValidUsername { sub _logLogin { my $self = shift; $self->session->db->write("insert into userLoginLog values (".$self->session->db->quote($_[0]).",".$self->session->db->quote($_[1]).",".$self->session->datetime->time()."," - .$self->session->db->quote($self->session->env->get("REMOTE_ADDR")).",".$self->session->db->quote($self->session->env->get("HTTP_USER_AGENT")).")"); + .$self->session->db->quote($self->session->env->getIp).",".$self->session->db->quote($self->session->env->get("HTTP_USER_AGENT")).")"); } diff --git a/lib/WebGUI/Session/Env.pm b/lib/WebGUI/Session/Env.pm index 2ac124933..00ce8e52a 100644 --- a/lib/WebGUI/Session/Env.pm +++ b/lib/WebGUI/Session/Env.pm @@ -66,13 +66,26 @@ The name of the variable. sub get { my $self = shift; my $var = shift; - if ($var eq "REMOTE_ADDR" && $self->{_env}{HTTP_X_FORWARDED_FOR} ne "") { - return $self->{_env}{HTTP_X_FORWARDED_FOR}; - } return $self->{_env}{$var}; } +#------------------------------------------------------------------- + +=head2 getIp ( ) + +Returns the user's real IP address. Normally this is REMOTE_ADDR, but if they go through a proxy server it might be in HTTP_X_FORWARDED_FOR. This method attempts to figure out what the most likely IP is for the user. Note that it's possible to spoof this and therefore shouldn't be used as your only security mechanism for validating a user. +=cut + +sub getIp { + my $self = shift; + if ($self->get("HTTP_X_FORWARDED_FOR") =~ m/(\d+\.\d+\.\d+\.\d+)/) { + return $1; + } + return $self->get("REMOTE_ADDR"); +} + + #------------------------------------------------------------------- =head2 new ( ) diff --git a/lib/WebGUI/Session/ErrorHandler.pm b/lib/WebGUI/Session/ErrorHandler.pm index 3952e11a8..0fdab7dad 100644 --- a/lib/WebGUI/Session/ErrorHandler.pm +++ b/lib/WebGUI/Session/ErrorHandler.pm @@ -90,7 +90,7 @@ sub canShowDebug { my $ips = $self->session->setting->get("debugIp"); $ips =~ s/\s+//g; my @ips = split(",", $ips); - my $ok = WebGUI::Utility::isInSubnet($self->session->env->get("REMOTE_ADDR"), [ @ips] ); + my $ok = WebGUI::Utility::isInSubnet($self->session->env->getIp, [ @ips] ); return $ok; } @@ -105,7 +105,7 @@ Returns true if the user meets the conditions to see performance indicators and sub canShowPerformanceIndicators { my $self = shift; my $mask = $self->session->setting->get("debugIp"); - my $ip = $self->session->env->get("REMOTE_ADDR"); + my $ip = $self->session->env->getIp; return ( ( $self->session->setting->get("showPerformanceIndicators") @@ -331,7 +331,7 @@ sub security { my $self = shift; my $message = shift; $self->warn($self->session->user->username." (".$self->session->user->userId.") connecting from " - .$self->session->env->get("REMOTE_ADDR")." attempted to ".$message); + .$self->session->env->getIp." attempted to ".$message); } diff --git a/lib/WebGUI/Session/Var.pm b/lib/WebGUI/Session/Var.pm index a20da819a..1c13ff2a9 100644 --- a/lib/WebGUI/Session/Var.pm +++ b/lib/WebGUI/Session/Var.pm @@ -181,7 +181,7 @@ sub new { $self->start(1,$sessionId); } elsif ($self->{_var}{sessionId} ne "") { $self->{_var}{lastPageView} = $session->datetime->time(); - $self->{_var}{lastIP} = $session->env->get("REMOTE_ADDR"); + $self->{_var}{lastIP} = $session->env->getIp; $self->{_var}{expires} = $session->datetime->time() + $session->setting->get("sessionTimeout"); $self->session->{_sessionId} = $self->{_var}{sessionId}; $session->db->setRow("userSession","sessionId",$self->{_var}); @@ -232,7 +232,7 @@ sub start { $self->{_var} = { expires=>$self->session->datetime->time() + $self->session->setting->get("sessionTimeout"), lastPageView=>$self->session->datetime->time(), - lastIP => $self->session->env->get("REMOTE_ADDR"), + lastIP => $self->session->env->getIp, adminOn => 0, userId => $userId };