Merge pull request #1634 from maggienegm/csrf-fix-forms

CSRF Fixes and Remote Code Execution Prevention
LibreHealthIO · Jul 19, 2021 · ec27daa · ec27daa
2 parents f249e0a + 9cf84f6
commit ec27daa
Show file tree

Hide file tree

Showing 70 changed files with 602 additions and 72 deletions.
diff --git a/controllers/C_Document.class.php b/controllers/C_Document.class.php
@@ -14,6 +14,7 @@
 require_once(dirname(__FILE__) . "/../library/forms.inc");
 require_once(dirname(__FILE__) . "/../library/formatting.inc.php");
 require_once(dirname(__FILE__) . "/../library/classes/postmaster.php" );
+require_once(dirname(__FILE__) . "/../library/sanitize.inc.php" );
 
 class C_Document extends Controller {
 
@@ -170,6 +171,8 @@ function upload_action_process() {
                     if ($_FILES['file']['size'][$key] == 0) {
                         $error .= "The system does not permit uploading files of with size 0.\n";
                     }
+                } elseif (!isSafeFile($_FILES['file']['tmp_name'][$key])) {
+                    $error = xl("The system does not permit uploading files with MIME content type") . " - " . mime_content_type($_FILES['file']['tmp_name'][$key]) . ".\n";
                 } else {
                     $tmpfile = fopen($_FILES['file']['tmp_name'][$key], "r");
                     $filetext = fread($tmpfile, $_FILES['file']['size'][$key]);

diff --git a/interface/billing/edi_history_main.php b/interface/billing/edi_history_main.php
@@ -70,6 +70,7 @@
 require_once("$srcdir/edihistory/ibr_ack_read.php");          //dirname(__FILE__) . "/edihist/ibr_ack_read.php");
 require_once("$srcdir/edihistory/ibr_uploads.php");           //dirname(__FILE__) . "/edihist/ibr_uploads.php");
 require_once("$srcdir/edihistory/ibr_io.php");                //dirname(__FILE__) . "/edihist/ibr_io.php");
+require_once("../../library/CsrfToken.php");
 //
 // php may output line endings if include files are utf-8
 ob_clean();
@@ -100,6 +101,14 @@
  */
 
 if (strtolower($_SERVER['REQUEST_METHOD']) == 'post') {
+    if (!empty($_POST)) {
+        if (!isset($_POST['token'])) {
+            error_log('WARNING: A POST request detected with no csrf token found');
+            die('Authentication failed.');
+        } else if (!(CsrfToken::verifyCsrfToken($_POST['token']))) {
+            die('Authentication failed.');
+        }
+    }
     //
     if ( isset($_POST['NewFiles']) ) {
         // process new files button clicked

diff --git a/interface/billing/edih_view.php b/interface/billing/edih_view.php
@@ -75,6 +75,7 @@
                 <legend><?php echo xlt("Select one or more files to upload"); ?></legend>
                 <input id="upload_file" type="file" name="fileUplMulti[]" class="cp-positive" multiple />
                 <input type="submit" name="uplsubmt" class="cp-submit" value="<?php echo xla("Submit"); ?>" />
+                <input type='hidden' name='token' value="<?php echo $_SESSION['token'];?>" />
                 </fieldset>
             </form>
          </td>
@@ -87,6 +88,7 @@
                 <input type="hidden" name="NewFiles" value="ProcessNew">
                 <label for="New-Files">Process New Files:</label>
                 <input id="processfiles"  name="Process" class="cp-output" type="button" value="<?php echo xla("Process"); ?>" />
+                <input type='hidden' name='token' value="<?php echo $_SESSION['token'];?>" />
                 </fieldset>
             </form>
          </td>
@@ -159,6 +161,7 @@
                         -->
                         <td align='center'>
                             <input type="hidden" name="csvshowtable" value="gettable">
+                            <input type='hidden' name='token' value="<?php echo $_SESSION['token'];?>" />
                             <input id="showtable" type="button" class="cp-submit" value="<?php echo xla("Submit"); ?>" />
                         </td>
 
@@ -220,6 +223,7 @@
                 <label for="era_file"><?php echo xlt("Filename"); ?>:</label>
                 <input id="era_file" type="file" size=20 name="fileUplEra" class="cp-positive" />
                 <input type="submit" name="fileERA" class="cp-submit" value="<?php echo xla("Submit"); ?>" />
+                <input type='hidden' name='token' value="<?php echo $_SESSION['token'];?>" />
             </fieldset>
             </form>
         </td>
@@ -236,6 +240,7 @@
             <label for="trace835"><?php echo xlt("Check No"); ?>:</label>
             <input type="text" size=10 name="trace835" value="" />
             <input type="submit" name="subtrace835" class="cp-submit" value="<?php echo xla("Submit"); ?>" />
+            <input type='hidden' name='token' value="<?php echo $_SESSION['token'];?>" />
         </fieldset>
         </form>
         </td>
@@ -253,6 +258,7 @@
                         <label for="enctr"><?php echo xlt("Enter Encounter"); ?>:</label>
                         <input type="text" name="enctrbatch" size=10 value="" />
                         <input type="submit" name="Batch-enctr" class="cp-submit" value="<?php echo xla("Submit"); ?>" />
+                        <input type='hidden' name='token' value="<?php echo $_SESSION['token'];?>" />
                     </fieldset>
                 </form>
               </td>
@@ -263,6 +269,7 @@
                     <label for="enctrERA"><?php echo xlt("Enter Encounter"); ?>:</label>
                     <input type="text" name="enctrEra" size=10 value="" />
                     <input type="submit" name="eraText" class="cp-submit" value="<?php echo xla("Submit"); ?>" />
+                    <input type='hidden' name='token' value="<?php echo $_SESSION['token'];?>" />
                 </fieldset>
                 </form>
               </td>
@@ -275,6 +282,7 @@
                     <label for="x12file"><?php echo xlt("Choose File"); ?>:</label>
                     <input id="x12file" type="file" name="fileUplx12" class="cp-positive" />
                     <input type="submit" name="fileX12" class="cp-submit" value="<?php echo xla("Submit"); ?>" />
+                    <input type='hidden' name='token' value="<?php echo $_SESSION['token'];?>" />
                 </fieldset>
                 </form>
             </td>
@@ -309,6 +317,7 @@
                     <input id="savenotes" type="button" class="cp-submit" value="<?php echo xla("Save"); ?>" />
                     <label for="closenotes"><?php echo xlt("Close"); ?></label>
                     <input id="closenotes" type="button" class="cp-negative" value="<?php echo xla("Close"); ?>" />
+                    <input type='hidden' name='token' value="<?php echo $_SESSION['token'];?>" />
                     </fieldset>
                     </form>
                 </td>

diff --git a/interface/billing/sl_eob_search.php b/interface/billing/sl_eob_search.php
@@ -470,7 +470,7 @@ function upload_file_to_client_pdf($file_to_send) {
       if ($DEBUG) {
         $alertmsg = xl("Printing skipped; see test output in") .' '. $STMT_TEMP_FILE;
       } else {
-        exec("$STMT_PRINT_CMD $STMT_TEMP_FILE");
+        exec(escapeshellcmd($STMT_PRINT_CMD) . " " . escapeshellarg($STMT_TEMP_FILE));
         if ($_POST['form_without']) {
           $alertmsg = xl('Now printing') .' '. $stmt_count .' '. xl('statements; invoices will not be updated.');
         } else {
@@ -665,7 +665,7 @@ function npopup(pid) {
     // Handle .zip extension if present.  Probably won't work on Windows.
     if (strtolower(substr($_FILES['form_erafile']['name'], -4)) == '.zip') {
       rename($tmp_name, "$tmp_name.zip");
-      exec("unzip -p $tmp_name.zip > $tmp_name");
+      exec("unzip -p " . escapeshellarg($tmp_name . ".zip") . " > " . escapeshellarg($tmp_name));
       unlink("$tmp_name.zip");
     }
 

diff --git a/interface/fax/fax_dispatch.php b/interface/fax/fax_dispatch.php
@@ -64,7 +64,7 @@
 //
 function getKittens($catid, $catstring, &$categories) {
   $cres = sqlStatement("SELECT id, name FROM categories " .
-    "WHERE parent = $catid ORDER BY name");
+    "WHERE parent = ? ORDER BY name", array($catid));
   $childcount = 0;
   while ($crow = sqlFetchArray($cres)) {
     ++$childcount;
@@ -88,7 +88,7 @@ function mergeTiffs() {
     $inames .= ' ' . escapeshellarg("$inbase.tif");
   }
   if (!$inames) die(xl("Internal error - no pages were selected!"));
-  $tmp0 = exec("cd '$faxcache'; tiffcp $inames temp.tif", $tmp1, $tmp2);
+  $tmp0 = exec("cd " . escapeshellarg($faxcache) . "; tiffcp $inames temp.tif", $tmp1, $tmp2);
   if ($tmp2) {
     $msg .= "tiffcp returned $tmp2: $tmp0 ";
   }
@@ -107,7 +107,7 @@ function mergeTiffs() {
     if (!$patient_id) die(xl('Internal error - patient ID was not provided!'));
     // Compute the name of the target directory and make sure it exists.
     $docdir = $GLOBALS['OE_SITE_DIR'] . "/documents/$patient_id";
-    exec("mkdir -p '$docdir'");
+    exec("mkdir -p " . escapeshellarg($docdir));
 
     // If copying to patient documents...
     //
@@ -134,7 +134,7 @@ function mergeTiffs() {
       $info_msg .= mergeTiffs();
       // The -j option here requires that libtiff is configured with libjpeg.
       // It could be omitted, but the output PDFs would then be quite large.
-      $tmp0 = exec("tiff2pdf -j -p letter -o '$target' '$faxcache/temp.tif'", $tmp1, $tmp2);
+      $tmp0 = exec("tiff2pdf -j -p letter -o " . escapeshellarg($target) . " " . escapeshellarg($faxcache . '/temp.tif'), $tmp1, $tmp2);
 
       if ($tmp2) {
         $info_msg .= "tiff2pdf returned $tmp2: $tmp0 ";
@@ -153,10 +153,8 @@ function mergeTiffs() {
         sqlStatement($query);
         $query = "INSERT INTO categories_to_documents ( " .
           "category_id, document_id" .
-          " ) VALUES ( " .
-          "'$catid', '$newid' " .
-          ")";
-        sqlStatement($query);
+          " ) VALUES (?, ?)";
+        sqlStatement($query, array($catid, $newid));
       } // end not error
 
       // If we are posting a note...
@@ -203,24 +201,22 @@ function mergeTiffs() {
 // scanned notes must be installed, and does not natively exist.
         $query = "INSERT INTO form_scanned_notes ( " .
           "notes " .
-          ") VALUES ( " .
-          "'" . $_POST['form_copy_sn_comments'] . "' " .
-          ")";
-        $formid = sqlInsert($query);
+          ") VALUES (?)";
+        $formid = sqlInsert($query, array($_POST['form_copy_sn_comments']));
         addForm($encounter_id, "Scanned Notes", $formid, "scanned_notes",
           $patient_id, $userauthorized);
         //
         $imagedir = $GLOBALS['OE_SITE_DIR'] . "/documents/$patient_id/encounters";
         $imagepath = "$imagedir/${encounter_id}_$formid.jpg";
         if (! is_dir($imagedir)) {
-          $tmp0 = exec('mkdir -p "' . $imagedir . '"', $tmp1, $tmp2);
+          $tmp0 = exec('mkdir -p ' . escapeshellarg($imagedir), $tmp1, $tmp2);
           if ($tmp2) die("mkdir returned $tmp2: $tmp0");
-          exec("touch '$imagedir/index.html'");
+          exec("touch " . escapeshellarg($imagedir . "/index.html"));
         }
         if (is_file($imagepath)) unlink($imagepath);
         // TBD: There may be a faster way to create this file, given that
         // we already have a jpeg for each page in faxcache.
-        $cmd = "convert -resize 800 -density 96 '$tmp_name' -append '$imagepath'";
+        $cmd = "convert -resize 800 -density 96 " . escapeshellarg($tmp_name) . " -append " . escapeshellarg($imagepath);
         $tmp0 = exec($cmd, $tmp1, $tmp2);
         if ($tmp2) die("\"$cmd\" returned $tmp2: $tmp0");
       }
@@ -271,17 +267,17 @@ function mergeTiffs() {
     $cpstring = str_replace('{MESSAGE}'       , $form_message , $cpstring);
     fwrite($tmph, $cpstring);
     fclose($tmph);
-    $tmp0 = exec("cd $webserver_root/custom; " . $GLOBALS['hylafax_enscript'] .
-      " -o $tmpfn2 $tmpfn1", $tmp1, $tmp2);
+      $tmp0 = exec("cd " . escapeshellarg($webserver_root . '/custom') . "; " . escapeshellcmd($GLOBALS['hylafax_enscript']) .
+        " -o " . escapeshellarg($tmpfn2) . " " . escapeshellarg($tmpfn1), $tmp1, $tmp2);
     if ($tmp2) {
       $info_msg .= "enscript returned $tmp2: $tmp0 ";
     }
     unlink($tmpfn1);
 
     // Send the fax as the cover page followed by the selected pages.
     $info_msg .= mergeTiffs();
-    $tmp0 = exec("sendfax -A -n $form_finemode -d " .
-      escapeshellarg($form_fax) . " $tmpfn2 '$faxcache/temp.tif'",
+    $tmp0 = exec("sendfax -A -n " . escapeshellarg($form_finemode) . " -d " .
+      escapeshellarg($form_fax) . " " . escapeshellarg($tmpfn2) . " " . escapeshellarg($faxcache . '/temp.tif'),
       $tmp1, $tmp2);
     if ($tmp2) {
       $info_msg .= "sendfax returned $tmp2: $tmp0 ";
@@ -356,21 +352,21 @@ function mergeTiffs() {
 // This will contain a .tif image as well as a .jpg image for each page.
 //
 if (! is_dir($faxcache)) {
-  $tmp0 = exec('mkdir -p "' . $faxcache . '"', $tmp1, $tmp2);
+  $tmp0 = exec('mkdir -p ' . escapeshellarg($faxcache), $tmp1, $tmp2);
   if ($tmp2) die("mkdir returned $tmp2: $tmp0");
   if (strtolower($ext) != '.tif') {
     // convert's default density for PDF-to-TIFF conversion is 72 dpi which is
     // not very good, so we upgrade it to "fine mode" fax quality.  It's really
     // better and faster if the scanner produces TIFFs instead of PDFs.
-    $tmp0 = exec("convert -density 203x196 '$filepath' '$faxcache/deleteme.tif'", $tmp1, $tmp2);
+    $tmp0 = exec("convert -density 203x196 " . escapeshellarg($filepath) . " " . escapeshellarg($faxcache . '/deleteme.tif'), $tmp1, $tmp2);
     if ($tmp2) die("convert returned $tmp2: $tmp0");
-    $tmp0 = exec("cd '$faxcache'; tiffsplit 'deleteme.tif'; rm -f 'deleteme.tif'", $tmp1, $tmp2);
+    $tmp0 = exec("cd " . escapeshellarg($faxcache) . "; tiffsplit 'deleteme.tif'; rm -f 'deleteme.tif'", $tmp1, $tmp2);
     if ($tmp2) die("tiffsplit/rm returned $tmp2: $tmp0");
   } else {
-    $tmp0 = exec("cd '$faxcache'; tiffsplit '$filepath'", $tmp1, $tmp2);
+    $tmp0 = exec("cd " . escapeshellarg($faxcache) . "; tiffsplit " . escapeshellarg($filepath), $tmp1, $tmp2);
     if ($tmp2) die("tiffsplit returned $tmp2: $tmp0");
   }
-  $tmp0 = exec("cd '$faxcache'; mogrify -resize 750x970 -format jpg *.tif", $tmp1, $tmp2);
+  $tmp0 = exec("cd " . escapeshellarg($faxcache) . "; mogrify -resize 750x970 -format jpg *.tif", $tmp1, $tmp2);
   if ($tmp2) die("mogrify returned $tmp2: $tmp0; ext is '$ext'; filepath is '$filepath'");
 }
 

diff --git a/interface/fax/faxq.php b/interface/fax/faxq.php
@@ -27,7 +27,7 @@
  if ($GLOBALS['enable_hylafax']) {
   // Get the recvq entries, parse and sort by filename.
   $statlines = array();
-  exec("faxstat -r -l -h " . $GLOBALS['hylafax_server'], $statlines);
+  exec("faxstat -r -l -h " . escapeshellarg($GLOBALS['hylafax_server']), $statlines);
   foreach ($statlines as $line) {
    // This gets pagecount, sender, time, filename.  We are expecting the
    // string to start with "-rw-rw-" so as to exclude faxes not yet fully
@@ -46,7 +46,7 @@
   154  124 F nobody 6153551807    0:1   4:12         No carrier detected
   */
   $donelines = array();
-  exec("faxstat -s -d -l -h " . $GLOBALS['hylafax_server'], $donelines);
+  exec("faxstat -s -d -l -h " . escapeshellarg($GLOBALS['hylafax_server']), $donelines);
   foreach ($donelines as $line) {
    // This gets jobid, priority, statchar, owner, phone, pages, dials and tts/status.
    if (preg_match('/^(\d+)\s+(\d+)\s+(\S)\s+(\S+)\s+(\S+)\s+(\d+:\d+)\s+(\d+:\d+)(.*)$/', $line, $matches)) {

diff --git a/interface/main/main_screen.php b/interface/main/main_screen.php
@@ -24,6 +24,7 @@
 /* Include our required headers */
 require_once('../globals.php');
 require_once("$srcdir/formdata.inc.php");
+require_once("../../library/CsrfToken.php");
 
 // Creates a new session id when load this outer frame
 // (allows creations of separate LibreHealth EHR frames to view patients concurrently
@@ -41,6 +42,9 @@
   session_regenerate_id(false);
 }
 
+//generate csrf token
+$_SESSION['token'] = CsrfToken::generateCsrfToken();
+
 $_SESSION["encounter"] = '';
 
 // Fetch the password expiration date

diff --git a/interface/main/tabs/js/tabs_view_model.js b/interface/main/tabs/js/tabs_view_model.js
@@ -249,7 +249,7 @@ function clearPatient()
     $.ajax({
         type: "POST",
         url: webroot_url+"/library/ajax/unset_session_ajax.php",
-	  data: { func: "unset_pid"},
+	  data: { func: "unset_pid", token: jsCsrfToken},
 	  success:function( msg ) {
 
 

diff --git a/interface/main/tabs/main.php b/interface/main/tabs/main.php
@@ -216,6 +216,8 @@ function isEncounterLocked( encounterId ) {
                                                                   .',' . json_encode($userQuery['fname'])
                                                                   .',' . json_encode($userQuery['lname'])
                                                                   .',' . json_encode($_SESSION['authGroup']); ?>));
+    // Set the csrf token used in js/tabs_view_model.js script
+    var jsCsrfToken = <?php echo $_SESSION['token'];?>;
 </script>
 
 <style type="text/css">

diff --git a/interface/patient_file/encounter/forms.php b/interface/patient_file/encounter/forms.php
@@ -106,6 +106,7 @@
             { amc_id: "patient_edu_amc",
               complete: true,
               mode: mode,
+              token: "<?php echo $_SESSION['token'];?>",
               patient_id: <?php echo htmlspecialchars($pid,ENT_NOQUOTES); ?>,
               object_category: "form_encounter",
               object_id: <?php echo htmlspecialchars($encounter,ENT_NOQUOTES); ?>
@@ -125,6 +126,7 @@
             { amc_id: "provide_sum_pat_amc",
               complete: true,
               mode: mode,
+              token: "<?php echo $_SESSION['token'];?>",
               patient_id: <?php echo htmlspecialchars($pid,ENT_NOQUOTES); ?>,
               object_category: "form_encounter",
               object_id: <?php echo htmlspecialchars($encounter,ENT_NOQUOTES); ?>
@@ -149,6 +151,7 @@
             { amc_id: "med_reconc_amc",
               complete: false,
               mode: mode,
+              token: "<?php echo $_SESSION['token'];?>",
               patient_id: <?php echo htmlspecialchars($pid,ENT_NOQUOTES); ?>,
               object_category: "form_encounter",
               object_id: <?php echo htmlspecialchars($encounter,ENT_NOQUOTES); ?>
@@ -168,6 +171,7 @@
             { amc_id: "med_reconc_amc",
               complete: true,
               mode: mode,
+              token: "<?php echo $_SESSION['token'];?>",
               patient_id: <?php echo htmlspecialchars($pid,ENT_NOQUOTES); ?>,
               object_category: "form_encounter",
               object_id: <?php echo htmlspecialchars($encounter,ENT_NOQUOTES); ?>

diff --git a/interface/patient_file/letter.php b/interface/patient_file/letter.php
@@ -33,6 +33,7 @@
 include_once($GLOBALS['srcdir'] . "/patient.inc");
 require_once($GLOBALS['srcdir']."/formatting.inc.php");
 require_once("$srcdir/headers.inc.php");
+require_once("$srcdir/CsrfToken.php");
 $DateFormat = DateFormatRead();
 $DateLocale = getLocaleCodeForDisplayLanguage($GLOBALS['language_default']);
 
@@ -90,6 +91,14 @@
 
 $alertmsg = ''; // anything here pops up in an alert box
 
+if (!empty($_POST)) {
+    if (!isset($_POST['token'])) {
+        error_log('WARNING: A POST request detected with no csrf token found');
+        die('Authentication failed.');
+    } else if (!(  CsrfToken::verifyCsrfTokenAndCompareHash($_POST['token'], '/letter.php.theform'))) {
+        die('Authentication failed.');
+    }
+}
 // If the Generate button was clicked...
 if ($_POST['formaction']=="generate") {
 
@@ -430,6 +439,7 @@ function insertAtCursor(myField, myValue) {
 <form method='post' action='letter.php' id="theform" name="theform">
 <input type="hidden" name="formaction" id="formaction" value="">
 <input type='hidden' name='form_pid' value='<?php echo $pid ?>' />
+<input type='hidden' name='token' value="<?php echo hash_hmac('sha256', (string) '/letter.php.theform', (string) $_SESSION['token']);?>" />
 
 <center>
 <p>