CVE-2023-0046: Improper Restriction of Names for Files and Other Resources (CWE-641)
value: Improper Restriction of Names for Files and Other Resources in GitHub repository lirantal/daloradius prior to master-branch.
CWE description: The product constructs the name of a file or other resource using input from an upstream component, but it does not restrict or incorrectly restricts the resulting name.
CVSS v3 base score: 7.2
Published: 2023-01-04T12:15Z
Repository: https://github.com/lirantal/daloradius
Language: PHP
Files changed: config-backup.php, config-logging.php
Commit date: 2023-01-04 11:43:01 +0100
Commit message: Better validating log filepath (#326)


Diff: diff --git a/config-backup.php b/config-backup.php
index 5c30ff20..26444c15 100644
--- a/config-backup.php
+++ b/config-backup.php
@@ -39,20 +39,10 @@
 
     include("menu-config-backup.php");
 
     echo '<div id="contentnorightbar">';
     print_title_and_help($title, $help);
-?>
-
-        </div><!-- #contentnorightbar -->
-        
-        <div id="footer">
-<?php
+    
     include('include/config/logging.php');
-    include('page-footer.php');
+    
+    print_footer_and_html_epilogue();
 ?>
-        </div><!-- #footer -->
-    </div>
-</div>
-
-</body>
-</html>
diff --git a/config-logging.php b/config-logging.php
index fc8cb835..df94868e 100644
--- a/config-logging.php
+++ b/config-logging.php
@@ -22,42 +22,93 @@
  */
 
     include ("library/checklogin.php");
     $operator = $_SESSION['operator_user'];
 
+    include('library/check_operator_perm.php');
     include_once('library/config_read.php');
+    
+    // init logging variables
     $log = "visited page: ";
-
+    $logAction = "";
+    $logDebugSQL = "";
+    
     include_once("lang/main.php");
+    include("library/validation.php");
+    include("library/layout.php");
 
     $param_label = array(
                             'CONFIG_LOG_PAGES' => t('all','PagesLogging'),
                             'CONFIG_LOG_QUERIES' => t('all','QueriesLogging'),
                             'CONFIG_LOG_ACTIONS' => t('all','ActionsLogging'),
                             'CONFIG_DEBUG_SQL' => t('all','LoggingDebugInfo'),
                             'CONFIG_DEBUG_SQL_ONPAGE' => t('all','LoggingDebugOnPages')
                         );
 
-    // if the form has been submitted we validate and store the configuration
-    if (array_key_exists('submit', $_POST) && isset($_POST['submit'])) {
-        
-        foreach ($param_label as $param => $label) {
-            if (array_key_exists($param, $_POST) && isset($_POST[$param]) &&
-                in_array(strtolower($_POST[$param]), array("yes", "no"))) {
-                $configValues[$param] = $_POST[$param];
+    if ($_SERVER['REQUEST_METHOD'] === 'POST') {
+        if (array_key_exists('csrf_token', $_POST) && isset($_POST['csrf_token']) && dalo_check_csrf_token($_POST['csrf_token'])) {
+            
+            $isError = false;
+            
+            // validate yes/no params
+            foreach ($param_label as $param => $label) {
+                if (array_key_exists($param, $_POST) && !empty(strtolower(trim($_POST[$param]))) &&
+                    in_array(strtolower(trim($_POST[$param])), array("yes", "no"))) {
+                    $configValues[$param] = $_POST[$param];
+                }
             }
+            
+            // validate path
+            $log_path_prefix = $configValues['CONFIG_PATH_DALO_VARIABLE_DATA'] . "/log";
+            $log_file_suffix = ".log";
+            
+            if (array_key_exists('CONFIG_LOG_FILE', $_POST) && !empty(trim($_POST['CONFIG_LOG_FILE']))) {
+                $candidate_log_file = trim($_POST['CONFIG_LOG_FILE']);
+                
+                if (
+                        // this ensure that the log_path_prefix is a directory
+                        is_dir($log_path_prefix) &&
+
+                        // this ensures that candidate_log_file starts with the log_path_prefix
+                        substr($candidate_log_file, 0, strlen($log_path_prefix)) === $log_path_prefix &&
+                        
+                        // this ensures that candidate_log_file does not contain ".."
+                        strpos($candidate_log_file, "..") === false &&
+                        
+                        // this ensures that candidate_log_file ends with the log_file_suffix
+                        substr($candidate_log_file, -strlen($log_file_suffix)) === $log_file_suffix &&
+                        
+                        // this ensures that candidate_log_file is at a writable location
+                        // or that at least it can be written inside the parent directory
+                        (is_writable($candidate_log_file) || is_writable(dirname($candidate_log_file)))
+                    ) {
+                    
+                    $configValues['CONFIG_LOG_FILE'] = $candidate_log_file;
+                    
+                } else {
+                    $isError = true;
+                }
+            } else {
+                $isError = true;
+            }
+            
+            // we write ONLY IF isError is false
+            if (!$isError) {
+                include("library/config_write.php");
+            } else {
+                $failureMsg = sprintf("Log path not allowed. Ensure that log file name locates a writable %s file contained in %s",
+                                      $log_file_suffix, $log_path_prefix);
+                $logAction .= "$failureMsg on page: ";
+            }
+            
+        } else {
+            // csrf
+            $failureMsg = "CSRF token error";
+            $logAction .= "$failureMsg on page: ";
         }
-
-        if (array_key_exists('CONFIG_LOG_FILE', $_POST) && isset($_POST['CONFIG_LOG_FILE']) &&
-            is_writable($_POST['CONFIG_LOG_FILE'])) {
-            $configValues['CONFIG_LOG_FILE'] = $_POST['CONFIG_LOG_FILE'];
-        }
-
-        include("library/config_write.php");
-    }    
-
-    include("library/layout.php");
+    }
+        
 
     // print HTML prologue
     $title = t('Intro','configlogging.php');
     $help = t('helpPage','configlogging');
     
@@ -67,47 +118,60 @@
 
     echo '<div id="contentnorightbar">';
     print_title_and_help($title, $help);
 
     include_once('include/management/actionMessages.php');
-
-?>
-
-<form name="loggingsettings" method="POST">
-    <fieldset>
-        <h302><?= t('title','Settings'); ?></h302>
-        <br>
-        
-        <ul>
-<?php
+    
+    $fieldset0_descriptor = array(
+                                    "title" => t('title','Settings')
+                                 );
+    
+    
+    $input_descriptors0 = array();
+    
     foreach ($param_label as $name => $label) {
-        print_select_as_list_elem($name, $label, array("no", "yes"), $configValues[$name]);
+        $input_descriptors0[] = array(
+                                        "type" => "select",
+                                        "options" => array( "yes", "no" ),
+                                        "caption" => $label,
+                                        "name" => $name,
+                                        "selected_value" => $configValues[$name]
+                                     );
     }
-?>
-
-            <li class="fieldset">
-                <label for="CONFIG_LOG_FILE" class="form"><?= t('all','FilenameLogging') ?></label>
-                <input value="<?= $configValues['CONFIG_LOG_FILE'] ?>" name="CONFIG_LOG_FILE" id="CONFIG_LOG_FILE">
-            </li>
-
-
-            <li class="fieldset">
-                <br/><hr><br/>
-                <input type="submit" name="submit" value="<?= t('buttons','apply') ?>" class="button">
-            </li>
-        </ul>
-    </fieldset>
-</form>
-
-        </div><!-- #contentnorightbar -->
-        
-        <div id="footer">
-<?php
+    
+    $input_descriptors0[] = array(
+                                        "type" => "text",
+                                        "caption" => t('all','FilenameLogging'),
+                                        "name" => 'CONFIG_LOG_FILE',
+                                        "value" => $configValues['CONFIG_LOG_FILE']
+                                     );
+    
+    $input_descriptors0[] = array(
+                                    "name" => "csrf_token",
+                                    "type" => "hidden",
+                                    "value" => dalo_csrf_token(),
+                                 );
+
+    $input_descriptors0[] = array(
+                                    'type' => 'submit',
+                                    'name' => 'submit',
+                                    'value' => t('buttons','apply')
+                                 );
+    
+    open_form();
+    
+    // open 0-th fieldset
+    open_fieldset($fieldset0_descriptor);
+    
+    foreach ($input_descriptors0 as $input_descriptor) {
+        print_form_component($input_descriptor);
+    }
+    
+    close_fieldset();
+    
+    close_form();
+    
     include('include/config/logging.php');
-    include('page-footer.php');
-?>
-        </div><!-- #footer -->
-    </div>
-</div>
+    
+    print_footer_and_html_epilogue();
 
-</body>
-</html>
+?>

Vulnerable code: ?>

        </div><!-- #contentnorightbar -->
        
        <div id="footer">
<?php
    include('page-footer.php');
        </div><!-- #footer -->
    </div>
</div>

</body>
</html>

    // if the form has been submitted we validate and store the configuration
    if (array_key_exists('submit', $_POST) && isset($_POST['submit'])) {
        
        foreach ($param_label as $param => $label) {
            if (array_key_exists($param, $_POST) && isset($_POST[$param]) &&
                in_array(strtolower($_POST[$param]), array("yes", "no"))) {
                $configValues[$param] = $_POST[$param];

        if (array_key_exists('CONFIG_LOG_FILE', $_POST) && isset($_POST['CONFIG_LOG_FILE']) &&
            is_writable($_POST['CONFIG_LOG_FILE'])) {
            $configValues['CONFIG_LOG_FILE'] = $_POST['CONFIG_LOG_FILE'];
        }

        include("library/config_write.php");
    }    

    include("library/layout.php");

?>

<form name="loggingsettings" method="POST">
    <fieldset>
        <h302><?= t('title','Settings'); ?></h302>
        <br>
        
        <ul>
<?php
        print_select_as_list_elem($name, $label, array("no", "yes"), $configValues[$name]);
?>

            <li class="fieldset">
                <label for="CONFIG_LOG_FILE" class="form"><?= t('all','FilenameLogging') ?></label>
                <input value="<?= $configValues['CONFIG_LOG_FILE'] ?>" name="CONFIG_LOG_FILE" id="CONFIG_LOG_FILE">
            </li>


            <li class="fieldset">
                <br/><hr><br/>
                <input type="submit" name="submit" value="<?= t('buttons','apply') ?>" class="button">
            </li>
        </ul>
    </fieldset>
</form>

        </div><!-- #contentnorightbar -->
        
        <div id="footer">
<?php
    include('page-footer.php');
?>
        </div><!-- #footer -->
    </div>
</div>
</body>
</html>
Fixed code:     
    
    print_footer_and_html_epilogue();
    include('library/check_operator_perm.php');
    
    // init logging variables
    $logAction = "";
    $logDebugSQL = "";
    
    include("library/validation.php");
    include("library/layout.php");
    if ($_SERVER['REQUEST_METHOD'] === 'POST') {
        if (array_key_exists('csrf_token', $_POST) && isset($_POST['csrf_token']) && dalo_check_csrf_token($_POST['csrf_token'])) {
            
            $isError = false;
            
            // validate yes/no params
            foreach ($param_label as $param => $label) {
                if (array_key_exists($param, $_POST) && !empty(strtolower(trim($_POST[$param]))) &&
                    in_array(strtolower(trim($_POST[$param])), array("yes", "no"))) {
                    $configValues[$param] = $_POST[$param];
                }
            
            // validate path
            $log_path_prefix = $configValues['CONFIG_PATH_DALO_VARIABLE_DATA'] . "/log";
            $log_file_suffix = ".log";
            
            if (array_key_exists('CONFIG_LOG_FILE', $_POST) && !empty(trim($_POST['CONFIG_LOG_FILE']))) {
                $candidate_log_file = trim($_POST['CONFIG_LOG_FILE']);
                
                if (
                        // this ensure that the log_path_prefix is a directory
                        is_dir($log_path_prefix) &&

                        // this ensures that candidate_log_file starts with the log_path_prefix
                        substr($candidate_log_file, 0, strlen($log_path_prefix)) === $log_path_prefix &&
                        
                        // this ensures that candidate_log_file does not contain ".."
                        strpos($candidate_log_file, "..") === false &&
                        
                        // this ensures that candidate_log_file ends with the log_file_suffix
                        substr($candidate_log_file, -strlen($log_file_suffix)) === $log_file_suffix &&
                        
                        // this ensures that candidate_log_file is at a writable location
                        // or that at least it can be written inside the parent directory
                        (is_writable($candidate_log_file) || is_writable(dirname($candidate_log_file)))
                    ) {
                    
                    $configValues['CONFIG_LOG_FILE'] = $candidate_log_file;
                    
                } else {
                    $isError = true;
                }
            } else {
                $isError = true;
            }
            
            // we write ONLY IF isError is false
            if (!$isError) {
                include("library/config_write.php");
            } else {
                $failureMsg = sprintf("Log path not allowed. Ensure that log file name locates a writable %s file contained in %s",
                                      $log_file_suffix, $log_path_prefix);
                $logAction .= "$failureMsg on page: ";
            }
            
        } else {
            // csrf
            $failureMsg = "CSRF token error";
            $logAction .= "$failureMsg on page: ";
    }
        
    
    $fieldset0_descriptor = array(
                                    "title" => t('title','Settings')
                                 );
    
    
    $input_descriptors0 = array();
    
        $input_descriptors0[] = array(
                                        "type" => "select",
                                        "options" => array( "yes", "no" ),
                                        "caption" => $label,
                                        "name" => $name,
                                        "selected_value" => $configValues[$name]
                                     );
    
    $input_descriptors0[] = array(
                                        "type" => "text",
                                        "caption" => t('all','FilenameLogging'),
                                        "name" => 'CONFIG_LOG_FILE',
                                        "value" => $configValues['CONFIG_LOG_FILE']
                                     );
    
    $input_descriptors0[] = array(
                                    "name" => "csrf_token",
                                    "type" => "hidden",
                                    "value" => dalo_csrf_token(),
                                 );

    $input_descriptors0[] = array(
                                    'type' => 'submit',
                                    'name' => 'submit',
                                    'value' => t('buttons','apply')
                                 );
    
    open_form();
    
    // open 0-th fieldset
    open_fieldset($fieldset0_descriptor);
    
    foreach ($input_descriptors0 as $input_descriptor) {
        print_form_component($input_descriptor);
    }
    
    close_fieldset();
    
    close_form();
    
    
    print_footer_and_html_epilogue();
?>