Fix a path traversal vulnerability

Fix a path traversal vulnerability (reported by  Graham Rymer
<gr348@cam.ac.uk>, Sun, 08 Mar 2015 15:03:09 +0000

Additioanlly, update CHANGES file to alos include an earlier,
ureleases unversioned change and a note about some (future) version
numbers being reserved.

CHANGES

@@ -1,3 +1,46 @@
+[Note - version numbers 0.6, 0.61, and 0.62 are reserved to refer to
+incompatible versions of this module that were distributed only as
+part of Raven authentication support for phpBB3 in early 2008]
+
+0.52 2015-03-10 JW
+
+Fix a path traversal vulnerability in handling keyid.
+
+The Ucam WebAuth response parameter 'keyid' identifies the RSA key
+used to sign the request. In this implementation keyid is used to form
+a file name from which the key is read. The implementation failed to
+sanitise the value of keyid, allowing an attacker to use path traversal
+to select a different key to use for verification. An attacker who
+could arrange for that key to be one for which he had the corresponding
+private key could effectively forge authentication messages.
+
+This edit checks that keyid matches the (recently-clarified)
+definition of being entirely numeric, of being no longer than 8
+characters, and of not starting with a zero.
+
+<unreleased> 2014-03-27 MCV
+
+Authenticate can now force re-authentication
+
+This is actually a batch of changes by John Sutton [DAMTP], which he
+describes as "enhancements and bug-fixes". The largest change he
+describes thus:
+
+    "The main function authenticate($authassertionid = NULL, $testauthonly
+     = NULL) now takes the two arguments:
+
+    $authassertionid - is used to force re-authentication.  If specified,
+    it can be read as “authenticate(‘but this ticket will not do’)”.  I
+    use it to implement an idle timeout as follows: on every successful
+    authentication, retrieve the current ticket id using the id() function
+    and store it somewhere.  When you want to force re-authentication,
+    pass this ticket id back in as the first argument to authenticate().
+
+    $testauthonly - a boolean which if set true means to only test the
+    authentication state, i.e., return true or false accordingly but do
+    NOT send any headers.  This is useful when you are working with ajax
+    requests."
+
 0.51 2008-04-28 JML 
 
  * Added support for (mandatory) hostname option, which can be set via the 

ucam_webauth.php

@@ -25,7 +25,7 @@
 //
 // $Id: ucam_webauth.php,v 1.3 2013/04/11 21:48:55 jes91 Exp $
 //
-// Version 0.51
+// Version 0.52
 
 class Ucam_Webauth {
 
@@ -296,21 +296,15 @@ function using_https() {
     return (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == "on");
   }
 
-  function load_key($key_id) {
-    $key_filename = $this->key_dir . '/' . $key_id . '.crt';
-    if (file_exists($key_filename)) {
-      $key_file = fopen($key_filename, 'r');
-      $key = fread($key_file, filesize($key_filename));
-      fclose($key_file);
-      return $key;
-    }
-    return NULL;
-  }
-
   function check_sig($data, $sig, $key_id) {
 
     // **** TODO : remove trailing slash from key_dir
 
+    // Ensure keyid only contains digits (protect against path traversal)
+    if (preg_match('/^[1-9][0-9]{0,7}$/', $key_id) !== 1) {
+      error_log('Invalid key identifier in response message',0);
+      return false;
+    }
     $key_filename = $this->key_dir . '/' . $key_id . '.crt';
     $key_file = fopen($key_filename, 'r');
     // Band-aid test for the most obvious cause of error - whole
@@ -536,15 +530,7 @@ function authenticate($authassertionid = NULL, $testauthonly = NULL) {
 
       $this->session_ticket[$this->SESSION_TICKET_STATUS] = '200';
       $this->session_ticket[$this->SESSION_TICKET_MSG] = '';
-      /*
-      $key = $this->load_key($token[$this->WLS_TOKEN_KEYID]);
 
-      if ($key == NULL) {
-	$this->session_ticket[$this->SESSION_TICKET_MSG] = 'Failed to load '.$this->key_dir . '/' . $token[$this->WLS_TOKEN_KEYID] . '.crt';
-	$this->session_ticket[$this->SESSION_TICKET_STATUS] = '600';
-	return $this->COMPLETE;
-      }
-      */
       //echo '<p>' . implode('!', $token) . '</p>';
       $sig = array_pop($token);
       $key_id = array_pop($token);