Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fit remote code execution in history and template imports functionality #1625

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fit remote code execution in history and template imports functionality #1625

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
60b0f36
begin work on csrf fixes
Ngai-E Jun 29, 2020
7633825
Created CsrfToken class
maggienegm Jul 2, 2020
a7102ca
fix remote code execution in on history and import template modules
Ngai-E Jul 15, 2020
09b50aa
Update CsrfToken.php
Ngai-E Jul 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions interface/billing/edi_history_main.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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'])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing a closing parenthesis here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @maggienegm. I am guessing you fixed in here PR #1634?? have not checked yet. If you did then I can just close this.

Copy link
Contributor

@maggienegm maggienegm Aug 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @Ngai-E , I fixed the closing parenthesis issues there

die('Authentication failed.');
}
}
//
if ( isset($_POST['NewFiles']) ) {
// process new files button clicked
Expand Down
9 changes: 9 additions & 0 deletions interface/billing/edih_view.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -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>

Expand Down Expand Up @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
4 changes: 4 additions & 0 deletions interface/main/main_screen.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -41,6 +42,9 @@
session_regenerate_id(false);
}

//generate csrf token
$_SESSION['token'] = CsrfToken::generateCsrfToken();

$_SESSION["encounter"] = '';

// Fetch the password expiration date
Expand Down
9 changes: 9 additions & 0 deletions interface/patient_file/letter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,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 (!hash_equals(hash_hmac('sha256', '/letter.php.theform', $_SESSION['token']), $_POST['token'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is verifyCsrfTokenAndCompareHash not used here rather?

die('Authentication failed.');
}
}
// If the Generate button was clicked...
if ($_POST['formaction']=="generate") {

Expand Down Expand Up @@ -430,6 +438,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', '/letter.php.theform', $_SESSION['token']);?>" />

<center>
<p>
Expand Down
45 changes: 45 additions & 0 deletions library/CsrfToken.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

class CsrfToken {

// Generate csrf token for authentication
function generateCsrfToken()
{
if (!extension_loaded('openssl')) {
error_log("ERROR: openssl extension not enabled, needed for proper functioning of LibreHealth");
die("Error: Systems needs openssl.");
}

$csrfToken = bin2hex(openssl_random_pseudo_bytes(32));

if (empty($csrfToken)) {
error_log("ERROR : Token generation failed");
die("Error : Unable to correctly generate token");
}

return $csrfToken;
}

// Function to verify a csrf token
function verifyCsrfToken($token)
{
if (hash_equals($_SESSION['token'], $token)) {
return true;
} else {
error_log("WARNING : Malicious attempt encountered");
return false;
}
}
// Function to verify a csrf token using with second token
function verifyCsrfTokenAndCompareHash($token, $secondToken)
{
if (hash_equals(hash_hmac('sha256', $secondToken, $_SESSION['token']), $token) {
Copy link
Contributor

@maggienegm maggienegm Aug 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing a closing parenthesis here too

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its necessary to type cast the inputs before supplying to hash_hmac, interesting things might occur if that is not done ( type juggling vulnerability ), if $secondToken is a user controlled input, if an array is passed, hash_hmac will return null.
For more details:
https://www.youtube.com/watch?v=MpeaSNERwQA

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @naveen17797 I will add that too.

return true;
} else {
error_log("WARNING : Malicious attempt encountered");
return false;
}
}
}

?>
1 change: 0 additions & 1 deletion library/sanitize.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,4 @@ function image_has_right_size($size) {
return $size < 20971520;
}


?>
9 changes: 9 additions & 0 deletions patient_portal/import_template.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,16 @@
$sanitize_all_escapes=true;
$fake_register_globals=false;
require_once("../interface/globals.php");
require_once("../library/CsrfToken.php");

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'])) {
Copy link
Contributor

@maggienegm maggienegm Aug 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing a closing parenthesis here too

die('Authentication failed.');
}
}
if($_POST['mode'] == 'get'){
echo file_get_contents($_POST['docid']);
exit;
Expand Down
3 changes: 2 additions & 1 deletion patient_portal/import_template_ui.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function getDocument(docname, mode, content){
$.ajax({
type: "POST",
url: liburl,
data: {docid: docname, mode: mode,content: content},
data: {docid: docname, mode: mode,content: content, token: <?php echo $_SESSION['token'];?>},
beforeSend: function(xhr){
console.log("Please wait..."+content);
},
Expand Down Expand Up @@ -192,6 +192,7 @@ function getDocument(docname, mode, content){
<button class="btn btn-primary" type="button" onclick="location.href='./patient/provider'"><?php echo xlt('Home'); ?></button>
<input type='hidden' name="up_dir" value='<?php global $getdir;
echo $getdir;?>' />
<input type='hidden' name='token' value="<?php echo $_SESSION['token'];?>" />
<button class="btn btn-success" type="submit" name="upload_submit" id="upload_submit"><?php echo xlt('Upload Template for'); ?> <span style="font-size:14px;" class="label label-default" id='ptstatus'></span></button>

</form>
Expand Down