Bug#700912: zoneminder: local file inclusion vulnerability

[Salvatore Bonaccorso]

Package: zoneminder
Version: 1.24.2-8
Severity: grave
Tags: security patch
Justification: user security hole
Control: fixed -1 1.25.0-1

Hi

In zoneminder forum there is the following security patch announce:

 http://www.zoneminder.com/forums/viewtopic.php?f=1&t=17979

1.24.2-8 is affected by this file inclusion vulnerability.

Attached are the patches from svn, r3483 and r3488.

Note: upstream 1.25.0 has a sligthly modified detaint function:

function detaintPath( $path )
{
    // Remove any absolute paths, or relative ones that want to go up
    $path = preg_replace( '/\.(?:\.+[\\/][\\/]*)+/', '', $path );
    $path = preg_replace( '/^[\\/]+/', '', $path );
    return( $path );
}

Regards
Salvatore

Index: web/includes/functions.php
===================================================================
--- web/includes/functions.php	(revision 3482)
+++ web/includes/functions.php	(revision 3483)
@@ -2350,13 +2350,21 @@
     return( rand( 1, 999999 ) );
 }
 
+function detaintPath( $path )
+{
+    // Remove any absolute paths, or relative ones that want to go up
+    $path = preg_replace( '/\.\.\//', '', $path );
+    $path = preg_replace( '/^\//', '', $path );
+    return( $path );
+}
+
 function getSkinFile( $file )
 {
     global $skinBase;
     $skinFile = false;
     foreach ( $skinBase as $skin )
     {
-        $tempSkinFile = 'skins'.'/'.$skin.'/'.$file;
+        $tempSkinFile = detaintPath( 'skins'.'/'.$skin.'/'.$file );
         if ( file_exists( $tempSkinFile ) )
             $skinFile = $tempSkinFile;
     }
@@ -2369,7 +2377,7 @@
     $skinFile = false;
     foreach ( $skinBase as $skin )
     {
-        $tempSkinFile = 'skins'.'/'.$skin.'/'.$file;
+        $tempSkinFile = detaintPath( 'skins'.'/'.$skin.'/'.$file );
         if ( file_exists( $tempSkinFile ) )
             $skinFile = $tempSkinFile;
     }
Index: web/index.php
===================================================================
--- web/index.php	(revision 3482)
+++ web/index.php	(revision 3483)
@@ -97,10 +97,13 @@
 require_once( 'includes/functions.php' );
 
 if ( isset($_REQUEST['view']) )
-    $view = validHtmlStr($_REQUEST['view']);
+    $view = detaintPath($_REQUEST['view']);
 
+if ( isset($_REQUEST['request']) )
+    $request = detaintPath($_REQUEST['request']);
+
 if ( isset($_REQUEST['action']) )
-    $action = validHtmlStr($_REQUEST['action']);
+    $action = detaintPath($_REQUEST['action']);
 
 require_once( 'includes/actions.php' );
 
@@ -109,13 +112,10 @@
 
 if ( isset( $_REQUEST['request'] ) )
 {
-    $request = validHtmlStr($_REQUEST['request']);
     foreach ( getSkinIncludes( 'ajax/'.$request.'.php', true, true ) as $includeFile )
     {
         if ( !file_exists( $includeFile ) )
-        {
             Fatal( "Request '$request' does not exist" );
-        }
         require_once $includeFile;
     }
     return;
@@ -127,9 +127,7 @@
         foreach ( $includeFiles as $includeFile )
         {
             if ( !file_exists( $includeFile ) )
-            {
                 Fatal( "View '$view' does not exist" );
-            }
             require_once $includeFile;
         }
     }

Index: web/includes/functions.php
===================================================================
--- web/includes/functions.php	(revision 3487)
+++ web/includes/functions.php	(revision 3488)
@@ -2353,8 +2353,8 @@
 function detaintPath( $path )
 {
     // Remove any absolute paths, or relative ones that want to go up
-    $path = preg_replace( '/\.\.\//', '', $path );
-    $path = preg_replace( '/^\//', '', $path );
+    $path = preg_replace( '/\.\.+\/\/*/', '', $path );
+    $path = preg_replace( '/^\/\/*/', '', $path );
     return( $path );
 }