Browse Source

Verify return_to URL correctly.

Fix bug with URLs when using HTTPS on port 80
Closes #2
tags/0.6
Chris Smith 14 years ago
parent
commit
649810a29b
2 changed files with 49 additions and 4 deletions
  1. 7
    0
      processor.php
  2. 42
    4
      urlbuilder.inc.php

+ 7
- 0
processor.php View File

@@ -279,6 +279,13 @@
279 279
  function processPositiveResponse($valid) {
280 280
   Logger::log('Positive response: identity = %s, expected = %s', $_REQUEST['openid_identity'], $_SESSION['openid']['claimedId']);
281 281
 
282
+  if (!URLBuilder::isValidReturnToURL($_REQUEST['openid_return_to'])) {
283
+   Logger::log('Return_to check failed: %s, URL: %s', $_REQUEST['openid_return_to'], URLBuilder::getCurrentURL(true));
284
+   error('diffreturnto', 'The identity provider stated return URL was '
285
+                         . $_REQUEST['openid_return_to'] . ' but it actually seems to be '
286
+                         . URLBuilder::getCurrentURL());
287
+  }
288
+
282 289
   if ($_REQUEST['openid_identity'] != $_SESSION['openid']['claimedId'] && $_REQUEST['openid_identity'] != $_SESSION['openid']['opLocalId']) {
283 290
    if ($_SESSION['openid']['claimedId'] == 'http://specs.openid.net/auth/2.0/identifier_select') {
284 291
     $disc = new Discoverer($_REQUEST['openid_claimed_id'], false);

+ 42
- 4
urlbuilder.inc.php View File

@@ -159,7 +159,43 @@
159 159
    return self::addArguments(false, $args);
160 160
   }
161 161
 
162
-  public static function getCurrentURL() {
162
+  public static function isValidReturnToURL($url) {
163
+   // 11.1: The URL scheme, authority, and path MUST be the same between the two URLs.
164
+   //       Any query parameters that are present in the "openid.return_to" URL MUST
165
+   //       also be present with the same values in the URL of the HTTP request the
166
+   //       RP received.
167
+
168
+   $actual = parse_url(self::getCurrentURL(true));
169
+   $return = parse_url($url);
170
+
171
+   foreach (array('scheme', 'host', 'port', 'user', 'pass', 'path') as $part) {
172
+    if ($part == 'port') {
173
+     if (!isset($actual['port'])) { $actual['port'] = $actual['scheme'] == 'https' ? 443 : 80; }
174
+     if (!isset($return['port'])) { $return['port'] = $return['scheme'] == 'https' ? 443 : 80; }
175
+    }
176
+
177
+    if (isset($actual[$part]) ^ isset($return[$part])) {
178
+     // Present in one but not the other
179
+     return false;
180
+    } else if (isset($actual[$part]) && $actual[$part] != $return[$part]) {
181
+     // Present in both but different
182
+     return false;
183
+    }
184
+   }
185
+
186
+   parse_str($actual['query'], $actualVars);
187
+   parse_str($return['query'], $returnVars);
188
+
189
+   foreach ($returnVars as $key => $value) {
190
+    if (!isset($actualVars[$key]) || $actualVars[$key] != $value) {
191
+     return false;
192
+    }
193
+   }
194
+ 
195
+   return true;
196
+  }
197
+
198
+  public static function getCurrentURL($raw = false) {
163 199
    $res = 'http';
164 200
 
165 201
    if (isset($_SERVER['HTTPS'])) {
@@ -168,14 +204,16 @@
168 204
 
169 205
    $res .= '://' . $_SERVER['SERVER_NAME'];
170 206
 
171
-   if ($_SERVER['SERVER_PORT'] != 80) {
207
+   if ($_SERVER['SERVER_PORT'] != (isset($_SERVER['HTTPS']) ? 443 : 80)) {
172 208
     $res .= ':' . $_SERVER['SERVER_PORT'];
173 209
    }
174 210
 
175 211
    $url = $_SERVER['REQUEST_URI'];
176 212
 
177
-   while (preg_match('/([\?&])openid[\._](.*?)=(.*?)(&|$)/', $url, $m)) {
178
-    $url = str_replace($m[0], $m[1], $url);
213
+   if (!$raw) {
214
+    while (preg_match('/([\?&])openid[\._](.*?)=(.*?)(&|$)/', $url, $m)) {
215
+     $url = str_replace($m[0], $m[1], $url);
216
+    }
179 217
    }
180 218
 
181 219
    $url = preg_replace('/\??&*$/', '', $url);

Loading…
Cancel
Save