Problem: Return URL will sometimes get messed up if the current URL contains a query string. If GetRequestUri() returns a URL with a query string containing the "&" character, the Handler receiving the redirect may get confused.

Solution: GetRequestUri() should be URL encoded.

Patch:Index: People.Class.Session.php
===================================================================
--- People.Class.Session.php (revision 93)
+++ People.Class.Session.php (working copy)
@@ -34,7 +34,7 @@
if ($this->UserID > 0 && !$this->User->PERMISSION_SIGN_IN) $this->End($Context->Authenticator);
$Url = AppendUrlParameters(
$Context->Configuration['SAFE_REDIRECT'],
- 'ReturnUrl='.GetRequestUri());
+ 'ReturnUrl='.urlencode(GetRequestUri()));
Redirect($Url);
}
}


Thanks for the sandwich :)

Here's a Framework function I put together to compare the host of the ReturnUrl to the Configuration's BASE_URL... (parse_url() documentation)<?php
// Function Name is not final!
function IsInternalUrl(&$Configuration, $FullUrl) {
$PathParts = parse_url($FullUrl);
$HostUrl = $PathParts['scheme'].'://'.$PathParts['host'].'/';
return strpos($Configuration['BASE_URL'], $HostUrl) === 0;
}
?>
This might solve the problem of hackers abusing the ReturnUrl parameter. The function would be called in People.Control.SignInForm.php right before the redirect.