CVE-2018-17180: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: An issue was discovered in OpenEMR before 5.0.1 Patch 7. Directory Traversal exists via docid=../ to /portal/lib/download_template.php.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 5.3
Published: 2019-05-17T16:29Z
Repository: https://github.com/openemr/openemr
Language: PHP
Files changed: portal/index.php, portal/lib/appsql.class.php, portal/lib/download_template.php
Commit date: 2018-08-12 00:49:28 -0700
Commit message: portal fixes (#1795)



Diff: diff --git a/portal/index.php b/portal/index.php
index a0b89fe302..e453e3eed1 100644
--- a/portal/index.php
+++ b/portal/index.php
@@ -347,11 +347,11 @@ if (!(isset($_SESSION['password_update']) || isset($_GET['requestNew']))) {
 <script type="text/javascript">
 $(document).ready(function() {
 
 <?php // if something went wrong
 if (isset($_GET['requestNew'])) {
-    $_SESSION['patient_portal_onsite_two'] = true;
+    $_SESSION['register'] = true;
     $_SESSION['authUser'] = 'portal-user';
     $_SESSION['pid'] = true;
     ?>
     $('.datepicker').datetimepicker({
         <?php $datetimepicker_timepicker = false; ?>
diff --git a/portal/lib/appsql.class.php b/portal/lib/appsql.class.php
index a90971baf4..07318e4df8 100644
--- a/portal/lib/appsql.class.php
+++ b/portal/lib/appsql.class.php
@@ -167,11 +167,11 @@ class ApplicationTable
                                 "VALUES (NOW(), ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
             } else {
                 $logsql = "update onsite_portal_activity set date=?, patient_id=?, activity=?, require_audit=?,".
                         "            pending_action=?, action_taken=?,status=?, narrative=?, table_action=?, table_args=?,".
                                         "action_user=?, action_taken_time=?, checksum=? ";
-                $logsql .= "where id=".$rec ." And patient_id=".$audit['patient_id'];
+                $logsql .= "where id='" . add_escape_custom($rec) . "' And patient_id='" . add_escape_custom($audit['patient_id']) . "'";
             }
 
             $return = sqlStatementNoLog($logsql, $audit);
             $result = true;
         } catch (Exception $e) {
diff --git a/portal/lib/download_template.php b/portal/lib/download_template.php
index f4585156e7..014793fa5a 100644
--- a/portal/lib/download_template.php
+++ b/portal/lib/download_template.php
@@ -126,14 +126,14 @@ function doSubs($s)
     global $tcnt, $grcnt, $ckcnt;
     global $html_flag;
     $nextLocation = 0;
     $groupLevel = 0;
     $groupCount = 0;
-    
+
     while (($keyLocation = strpos($s, '{', $nextLocation)) !== false) {
         $nextLocation = $keyLocation + 1;
-        
+
         if (keySearch($s, '{PatientSignature}')) {
             $fn = $GLOBALS['web_root'] . '/portal/sign/assets/signhere.png';
             $sigfld = '<span>';
             $sigfld .= '<img style="cursor:pointer;color:red" class="signature" type="patient-signature" id="patientSignature" onclick="getSignature(this)"' . 'alt="' . xla("Click in signature on file") . '" src="' . $fn . '">';
             $sigfld .= '</span>';
@@ -229,11 +229,11 @@ function doSubs($s)
         } else if (keySearch($s, '{ChiefComplaint}')) {
             $cc = $enrow['reason'];
             $patientid = $ptrow['pid'];
             $DOS = substr($enrow['date'], 0, 10);
             // Prefer appointment comment if one is present.
-            $evlist = fetchEvents($DOS, $DOS, " AND pc_pid = '$patientid' ");
+            $evlist = fetchEvents($DOS, $DOS, " AND pc_pid = ? ", null, false, 0, array($patientid));
             foreach ($evlist as $tmp) {
                 if ($tmp['pc_pid'] == $pid && ! empty($tmp['pc_hometext'])) {
                     $cc = $tmp['pc_hometext'];
                 }
             }
@@ -343,11 +343,11 @@ function doSubs($s)
             }
 
             $s = keyReplace($s, dataFixup($data, $title));
         }
     } // End if { character found.
-    
+
     return $s;
 }
 // Get patient demographic info.
 $ptrow = sqlQuery("SELECT pd.*, " . "ur.fname AS ur_fname, ur.mname AS ur_mname, ur.lname AS ur_lname, ur.title AS ur_title, ur.specialty AS ur_specialty " . "FROM patient_data AS pd " . "LEFT JOIN users AS ur ON ur.id = pd.ref_providerID " . "WHERE pd.pid = ?", array(
     $pid
@@ -366,13 +366,16 @@ if ($encounter) {
         $encounter
     ));
 }
 
 $templatedir = $GLOBALS['OE_SITE_DIR'] . '/documents/onsite_portal_documents/templates';
+
+check_file_dir_name($form_filename);
 $templatepath = "$templatedir/$form_filename";
 // test if this is folder with template, if not, must be for a specific patient
 if (! file_exists($templatepath)) {
+    check_file_dir_name($pid);
     $templatepath = "$templatedir/" . $pid . "/$form_filename";
 }
 
 // Create a temporary file to hold the output.
 $fname = tempnam($GLOBALS['temporary_files_dir'], 'OED');

Vulnerable code:     $_SESSION['patient_portal_onsite_two'] = true;
                $logsql .= "where id=".$rec ." And patient_id=".$audit['patient_id'];
    
        
            $evlist = fetchEvents($DOS, $DOS, " AND pc_pid = '$patientid' ");
    
Fixed code:     $_SESSION['register'] = true;
                $logsql .= "where id='" . add_escape_custom($rec) . "' And patient_id='" . add_escape_custom($audit['patient_id']) . "'";


            $evlist = fetchEvents($DOS, $DOS, " AND pc_pid = ? ", null, false, 0, array($patientid));


check_file_dir_name($form_filename);
    check_file_dir_name($pid);