Merge pull request #306 from ArthurHoaro/fpd

Avoid Full Path Disclosure error on session error.

3 files changed, 71 insertions, 15 deletions

diff --git a/application/Utils.php b/application/Utils.php
index cd4724fa..fa18f158 100644
--- a/application/Utils.php
+++ b/application/Utils.php
@@ -137,4 +137,28 @@ function checkPHPVersion($minVersion, $curVersion)
         );
     }
 }
-?>
+
+/**
+ * Validate session ID to prevent Full Path Disclosure.
+ * See #298.
+ *
+ * @param string $sessionId Session ID
+ *
+ * @return true if valid, false otherwise.
+ */
+function is_session_id_valid($sessionId)
+{
+    if (empty($sessionId)) {
+        return false;
+    }
+
+    if (!$sessionId) {
+        return false;
+    }
+
+    if (!preg_match('/^[a-z0-9]{2,32}$/', $sessionId)) {
+        return false;
+    }
+
+    return true;
+}
diff --git a/index.php b/index.php
index 8e04fa3e..a093a283 100755
--- a/index.php
+++ b/index.php
@@ -43,19 +43,6 @@ define('shaarli_version','0.5.1');
 // http://server.com/x/shaarli --> /shaarli/
 define('WEB_PATH', substr($_SERVER["REQUEST_URI"], 0, 1+strrpos($_SERVER["REQUEST_URI"], '/', 0)));
 
-// Force cookie path (but do not change lifetime)
-$cookie=session_get_cookie_params();
-$cookiedir = ''; if(dirname($_SERVER['SCRIPT_NAME'])!='/') $cookiedir=dirname($_SERVER["SCRIPT_NAME"]).'/';
-session_set_cookie_params($cookie['lifetime'],$cookiedir,$_SERVER['SERVER_NAME']); // Set default cookie expiration and path.
-
-// Set session parameters on server side.
-define('INACTIVITY_TIMEOUT',3600); // (in seconds). If the user does not access any page within this time, his/her session is considered expired.
-ini_set('session.use_cookies', 1);       // Use cookies to store session.
-ini_set('session.use_only_cookies', 1);  // Force cookies for session (phpsessionID forbidden in URL).
-ini_set('session.use_trans_sid', false); // Prevent PHP form using sessionID in URL if cookies are disabled.
-session_name('shaarli');
-if (session_id() == '') session_start();  // Start session if needed (Some server auto-start sessions).
-
 // PHP Settings
 ini_set('max_input_time','60');  // High execution time in case of problematic imports/exports.
 ini_set('memory_limit', '128M');  // Try to set max upload file size and read (May not work on some hosts).
@@ -87,6 +74,34 @@ try {
     exit;
 }
 
+// Force cookie path (but do not change lifetime)
+$cookie = session_get_cookie_params();
+$cookiedir = '';
+if (dirname($_SERVER['SCRIPT_NAME']) != '/') {
+    $cookiedir = dirname($_SERVER["SCRIPT_NAME"]).'/';
+}
+// Set default cookie expiration and path.
+session_set_cookie_params($cookie['lifetime'], $cookiedir, $_SERVER['SERVER_NAME']);
+// Set session parameters on server side.
+// If the user does not access any page within this time, his/her session is considered expired.
+define('INACTIVITY_TIMEOUT', 3600); // in seconds.
+// Use cookies to store session.
+ini_set('session.use_cookies', 1);
+// Force cookies for session (phpsessionID forbidden in URL).
+ini_set('session.use_only_cookies', 1);
+// Prevent PHP form using sessionID in URL if cookies are disabled.
+ini_set('session.use_trans_sid', false);
+
+// Regenerate session id if invalid or not defined in cookie.
+if (isset($_COOKIE['shaarli']) && !is_session_id_valid($_COOKIE['shaarli'])) {
+    $_COOKIE['shaarli'] = uniqid();
+}
+session_name('shaarli');
+// Start session if needed (Some server auto-start sessions).
+if (session_id() == '') {
+    session_start();
+}
+
 include "inc/rain.tpl.class.php"; //include Rain TPL
 raintpl::$tpl_dir = $GLOBALS['config']['RAINTPL_TPL']; // template directory
 raintpl::$cache_dir = $GLOBALS['config']['RAINTPL_TMP']; // cache directory
diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php
index 28e15f5a..e39ce6be 100644
--- a/tests/UtilsTest.php
+++ b/tests/UtilsTest.php
@@ -150,5 +150,22 @@ class UtilsTest extends PHPUnit_Framework_TestCase
     {
         checkPHPVersion('5.3', '5.2');
     }
+
+    /**
+     * Test is_session_id_valid with a valid ID.
+     */
+    public function testIsSessionIdValid()
+    {
+        $this->assertTrue(is_session_id_valid('123456789012345678901234567890az'));
+    }
+
+    /**
+     * Test is_session_id_valid with invalid IDs.
+     */
+    public function testIsSessionIdInvalid()
+    {
+        $this->assertFalse(is_session_id_valid(''));
+        $this->assertFalse(is_session_id_valid(array()));
+        $this->assertFalse(is_session_id_valid('c0ZqcWF3VFE2NmJBdm1HMVQ0ZHJ3UmZPbTFsNGhkNHI='));
+    }
 }
-?>