search

LibreHealthIO / lh-ehr

LibreHealth EHR - Free Open Source Electronic Health Records
Other
238 stars 262 forks source link

MySQL errors showing up in search submission #1438

Closed Ngai-E closed 5 years ago

Ngai-E commented 5 years ago

A) Outreachy Username: elizabeth

B) Issue Title: MySQL errors showing up in search submission exposing database structure of LibreHealth EHR.

C) Bug Report Date: March 13, 2019.

D) Site Affected: Documentation Site and NHANES

E) OS/ Browser Used: Chrome on Ubuntu 16.04

F) Which Workflow Module in LHEHR: Report - Sales by Item

G) Steps to Reproduce the Bug:

  1. Login to LibreHealth EHR.
  2. Select Reports menu item.
  3. Hover over Financial drop down option and click on Sales by Item.
  4. Choose a facility from the Facility drop down menu which is not –All Facilities--.
  5. Choose a From and To date range (Optional to reproduce bug).
  6. Click on Submit.
  7. Similarly, choose a provider from the Provider drop down which is different from –All Providers-- and repeat steps 5 and 6.

H) At Point of the Bug, the Expected Behavior: User expects to see a list of sold items displayed in an orderly manner, say, a table for example.

I) Details of What Actually Happened: User is presented with a bunch of MySQL error complaining of syntax in SQL statement and a dump of failed database querry.

J) Provide Relevant Screenshot and Explain the Screenshot: sqlerror Illustration 1: Submit button returns MySQL errors

K) Estimation of The Bug Severity: Module function continues with ongoing errors (Critical).

aethelwulffe commented 5 years ago

This is due to your server error reporting settings. This is not a bug, just a developer-level php.ini configuration issue.
I use something like this in my config // toggle this to change the setting define('DEBUG', true); // you want all errors to be triggered error_reporting(E_ALL);

if(DEBUG == true) { // you're developing, so you want all errors to be shown display_errors(true); // logging is usually overkill during dev log_errors(false); } else { // you don't want to display errors on a prod environment display_errors(false); // you definitely wanna log any occurring log_errors(true); } Otherwise just turn off display_errors in php.ini

aethelwulffe commented 5 years ago

What caused this is possibly a (currently) bad query, or lack of data without sanity checking database or globals configuration. I doubt the warehouse option is turned on in globals for the site you are looking at.

Ngai-E commented 5 years ago

What caused this is possibly a (currently) bad query, or lack of data without sanity checking database or globals configuration. I doubt the warehouse option is turned on in globals for the site you are looking at.

Thanks @aethelwulffe for the review but i tested this on the online documentation site for LibreHealth not my local deployment. That is why i reported it as a bug.

aethelwulffe commented 5 years ago

Hi Bee, I caught on to that. Something you might want to take up with the Librehealth.io admin. I have it in my testing que to see if this is related to an actual sql error, or just a site configuration boo-boo in addition to the show_error bit. On that basis though, it is not a security issue.

Ngai-E commented 5 years ago

Alright but @aethelwulffe it's a bug right?

aethelwulffe commented 5 years ago

I think so. Basically the array passed to the prepare_query() function in the controller seems to not be getting the full array of elements. My test was identical to yours, but against much older code. Exactly the same result. If you don't define a facility, everything is cool. If you do, then you get the error.

The related code from the controller (not where it is called or where the array comes from):

function prepareQuery() {
  global $from_date, $to_date, $form_facility, $form_provider;
  $sqlBindArray = array();
  $query = "SELECT b.fee, b.pid, b.encounter, b.code_type, b.code, b.units, " .
    "b.code_text, fe.date, fe.facility_id, fe.provider_id, fe.invoice_refno, lo.title " .
    "FROM billing AS b " .
    "JOIN code_types AS ct ON ct.ct_key = b.code_type " .
    "JOIN form_encounter AS fe ON fe.pid = b.pid AND fe.encounter = b.encounter " .
    "LEFT JOIN codes AS c ON c.code_type = ct.ct_id AND c.code = b.code AND c.modifier = b.modifier " .
    "LEFT JOIN list_options AS lo ON lo.list_id = 'superbill' AND lo.option_id = c.superbill " .
    "WHERE b.code_type != 'COPAY' AND b.activity = 1 AND b.fee != 0 AND " .
    "fe.date >= ? AND fe.date <= ?";
    array_push($sqlBindArray,$from_date,$to_date);
  // If a facility was specified.
  if ($form_facility) {
    $query .= " AND fe.facility_id = ?";
    array_push($sqlBindArray,$form_facility);
  }
  if ($form_provider) {
    $query .= " AND fe.provider_id = ?";
    array_push($sqlBindArray,$form_provider);
  }
  $query .= " ORDER BY lo.title, b.code, fe.date, fe.id";
  //
  $res = sqlStatement($query,$sqlBindArray);
  while ($row = sqlFetchArray($res)) {
    thisLineItem($row['pid'], $row['encounter'],
      $row['title'], $row['code'] . ' ' . $row['code_text'],
      substr($row['date'], 0, 10), $row['units'], $row['fee'], $row['invoice_refno']);
  }
  //
  $sqlBindArray = array();
  $query = "SELECT s.sale_date, s.fee, s.quantity, s.pid, s.encounter, " .
    "d.name, fe.date, fe.facility_id, fe.provider_id, fe.invoice_refno " .
    "FROM drug_sales AS s " .
    "JOIN drugs AS d ON d.drug_id = s.drug_id " .
    "JOIN form_encounter AS fe ON " .
    "fe.pid = s.pid AND fe.encounter = s.encounter AND " .
    "fe.date >= '$from_date 00:00:00' AND fe.date <= '$to_date 23:59:59' " .
    //"fe.date >= ? AND fe.date <= ? " .
    "WHERE s.fee != 0";
    array_push($sqlBindArray,$from_date,$to_date);
  // If a facility was specified.
  if ($form_facility) {
    $query .= " AND fe.facility_id = ?";
     array_push($sqlBindArray,$form_facility);
  }
  if ($form_provider) {
    $query .= " AND fe.provider_id = ?";
    array_push($sqlBindArray,$form_provider);
  }
  $query .= " ORDER BY d.name, fe.date, fe.id";
  //
  return $query;
}
Ngai-E commented 5 years ago

I think so. Basically the array passed to the prepare_query() function in the controller seems to not be getting the full array of elements. My test was identical to yours, but against much older code. Exactly the same result. If you don't define a facility, everything is cool. If you do, then you get the error.

The related code from the controller (not where it is called or where the array comes from):

function prepareQuery() {
  global $from_date, $to_date, $form_facility, $form_provider;
  $sqlBindArray = array();
  $query = "SELECT b.fee, b.pid, b.encounter, b.code_type, b.code, b.units, " .
    "b.code_text, fe.date, fe.facility_id, fe.provider_id, fe.invoice_refno, lo.title " .
    "FROM billing AS b " .
    "JOIN code_types AS ct ON ct.ct_key = b.code_type " .
    "JOIN form_encounter AS fe ON fe.pid = b.pid AND fe.encounter = b.encounter " .
    "LEFT JOIN codes AS c ON c.code_type = ct.ct_id AND c.code = b.code AND c.modifier = b.modifier " .
    "LEFT JOIN list_options AS lo ON lo.list_id = 'superbill' AND lo.option_id = c.superbill " .
    "WHERE b.code_type != 'COPAY' AND b.activity = 1 AND b.fee != 0 AND " .
    "fe.date >= ? AND fe.date <= ?";
    array_push($sqlBindArray,$from_date,$to_date);
  // If a facility was specified.
  if ($form_facility) {
    $query .= " AND fe.facility_id = ?";
    array_push($sqlBindArray,$form_facility);
  }
  if ($form_provider) {
    $query .= " AND fe.provider_id = ?";
    array_push($sqlBindArray,$form_provider);
  }
  $query .= " ORDER BY lo.title, b.code, fe.date, fe.id";
  //
  $res = sqlStatement($query,$sqlBindArray);
  while ($row = sqlFetchArray($res)) {
    thisLineItem($row['pid'], $row['encounter'],
      $row['title'], $row['code'] . ' ' . $row['code_text'],
      substr($row['date'], 0, 10), $row['units'], $row['fee'], $row['invoice_refno']);
  }
  //
  $sqlBindArray = array();
  $query = "SELECT s.sale_date, s.fee, s.quantity, s.pid, s.encounter, " .
    "d.name, fe.date, fe.facility_id, fe.provider_id, fe.invoice_refno " .
    "FROM drug_sales AS s " .
    "JOIN drugs AS d ON d.drug_id = s.drug_id " .
    "JOIN form_encounter AS fe ON " .
    "fe.pid = s.pid AND fe.encounter = s.encounter AND " .
    "fe.date >= '$from_date 00:00:00' AND fe.date <= '$to_date 23:59:59' " .
    //"fe.date >= ? AND fe.date <= ? " .
    "WHERE s.fee != 0";
    array_push($sqlBindArray,$from_date,$to_date);
  // If a facility was specified.
  if ($form_facility) {
    $query .= " AND fe.facility_id = ?";
     array_push($sqlBindArray,$form_facility);
  }
  if ($form_provider) {
    $query .= " AND fe.provider_id = ?";
    array_push($sqlBindArray,$form_provider);
  }
  $query .= " ORDER BY d.name, fe.date, fe.id";
  //
  return $query;
}

Yep that is my point. Thanks for confirming

aethelwulffe commented 5 years ago

Full (redacted for server stuff) error report on my end is:

[Wed Mar 13 12:53:37.385044 2019] [php7:notice] [pid 31538] [client 192.168.2.1:41639] PHP Notice:  Undefined variable: from_date in /var/www/xxxxxxxxx/library/report_functions.php on line 101, referer: https://xxxxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:53:37.386677 2019] [php7:notice] [pid 31538] [client 192.168.2.1:41639] PHP Notice:  Undefined variable: to_date in /var/www/xxxxxxxxx/library/report_functions.php on line 110, referer: https://xxxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:53:37.413446 2019] [php7:notice] [pid 31538] [client 192.168.2.1:41639] PHP Notice:  Undefined variable: form_from_date in /var/www/xxxxxxxx/interface/reports/sales_by_item.php on line 211, referer: https://xxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:53:37.413552 2019] [php7:notice] [pid 31538] [client 192.168.2.1:41639] PHP Notice:  Undefined variable: form_to_date in /var/www/xxxxxx/interface/reports/sales_by_item.php on line 212, referer: https://xxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:53:37.692991 2019] [php7:notice] [pid 31538] [client 192.168.2.1:41639] PHP Notice:  Undefined variable: sqlBindArray in /var/www/xxxxxxx/interface/reports/sales_by_item.php on line 226, referer: https://xxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.588215 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] PHP Notice:  Undefined variable: from_date in /var/www/xxxxxxxx/library/report_functions.php on line 101, referer: https://xxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.589678 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] PHP Notice:  Undefined variable: to_date in /var/www/xxxxxxxxx/library/report_functions.php on line 110, referer: https://xxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.614015 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] PHP Notice:  Undefined variable: form_from_date in /var/www/xxxxxxxxx/interface/reports/sales_by_item.php on line 211, referer: https://xxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.614113 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] PHP Notice:  Undefined variable: form_to_date in xxxxxxxxxxxy/interface/reports/sales_by_item.php on line 212, referer: https://xxxxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.867446 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] PHP Notice:  Undefined variable: sqlBindArray in /var/www/xxxxxxx/interface/reports/sales_by_item.php on line 226, referer: https://xxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.868997 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] SQL Error with statement:query failed: SELECT s.sale_date, s.fee, s.quantity, s.pid, s.encounter, d.name, fe.date, fe.facility_id, fe.provider_id, fe.invoice_refno FROM drug_sales AS s JOIN drugs AS d ON d.drug_id = s.drug_id JOIN form_encounter AS fe ON fe.pid = s.pid AND fe.encounter = s.encounter AND fe.date >= ' 00:00:00 00:00:00' AND fe.date <= ' 23:59:59 23:59:59' WHERE s.fee != 0 AND fe.facility_id = ? ORDER BY d.name, fe.date, fe.id--You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '? ORDER BY d.name, fe.date, fe.id' at line 1==>xxxxxxxx/interface/reports/sales_by_item.php at 226:sqlStatement, referer: https:xxxxxxxxxxx/interface/reports/sales_by_item.php

I see two things: No terminal semicolon, and the dates are NOT what I put in. Nor is 23:59:59 23:59:59 a valid date.

Ngai-E commented 5 years ago

Full (redacted for server stuff) error report on my end is:

[Wed Mar 13 12:53:37.385044 2019] [php7:notice] [pid 31538] [client 192.168.2.1:41639] PHP Notice:  Undefined variable: from_date in /var/www/xxxxxxxxx/library/report_functions.php on line 101, referer: https://xxxxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:53:37.386677 2019] [php7:notice] [pid 31538] [client 192.168.2.1:41639] PHP Notice:  Undefined variable: to_date in /var/www/xxxxxxxxx/library/report_functions.php on line 110, referer: https://xxxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:53:37.413446 2019] [php7:notice] [pid 31538] [client 192.168.2.1:41639] PHP Notice:  Undefined variable: form_from_date in /var/www/xxxxxxxx/interface/reports/sales_by_item.php on line 211, referer: https://xxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:53:37.413552 2019] [php7:notice] [pid 31538] [client 192.168.2.1:41639] PHP Notice:  Undefined variable: form_to_date in /var/www/xxxxxx/interface/reports/sales_by_item.php on line 212, referer: https://xxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:53:37.692991 2019] [php7:notice] [pid 31538] [client 192.168.2.1:41639] PHP Notice:  Undefined variable: sqlBindArray in /var/www/xxxxxxx/interface/reports/sales_by_item.php on line 226, referer: https://xxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.588215 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] PHP Notice:  Undefined variable: from_date in /var/www/xxxxxxxx/library/report_functions.php on line 101, referer: https://xxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.589678 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] PHP Notice:  Undefined variable: to_date in /var/www/xxxxxxxxx/library/report_functions.php on line 110, referer: https://xxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.614015 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] PHP Notice:  Undefined variable: form_from_date in /var/www/xxxxxxxxx/interface/reports/sales_by_item.php on line 211, referer: https://xxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.614113 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] PHP Notice:  Undefined variable: form_to_date in xxxxxxxxxxxy/interface/reports/sales_by_item.php on line 212, referer: https://xxxxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.867446 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] PHP Notice:  Undefined variable: sqlBindArray in /var/www/xxxxxxx/interface/reports/sales_by_item.php on line 226, referer: https://xxxxxxxxxx/interface/reports/sales_by_item.php
[Wed Mar 13 12:54:18.868997 2019] [php7:notice] [pid 26552] [client 192.168.2.1:30576] SQL Error with statement:query failed: SELECT s.sale_date, s.fee, s.quantity, s.pid, s.encounter, d.name, fe.date, fe.facility_id, fe.provider_id, fe.invoice_refno FROM drug_sales AS s JOIN drugs AS d ON d.drug_id = s.drug_id JOIN form_encounter AS fe ON fe.pid = s.pid AND fe.encounter = s.encounter AND fe.date >= ' 00:00:00 00:00:00' AND fe.date <= ' 23:59:59 23:59:59' WHERE s.fee != 0 AND fe.facility_id = ? ORDER BY d.name, fe.date, fe.id--You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '? ORDER BY d.name, fe.date, fe.id' at line 1==>xxxxxxxx/interface/reports/sales_by_item.php at 226:sqlStatement, referer: https:xxxxxxxxxxx/interface/reports/sales_by_item.php

I see two things: No terminal semicolon, and the dates are NOT what I put in. Nor is 23:59:59 23:59:59 a valid date.

Nope it is not a valid one. I also discovered that even without putting the date you still get the SQL error.

KoniKodes commented 5 years ago

So I'm confused. Is this an error, and how can we fix it?

Ngai-E commented 5 years ago

So I'm confused. Is this an error, and how can we fix it?

Ngai-E commented 5 years ago

I think @KoniKodes i have an idea how to go about the fix for the front end. I have worked on something similar before I will work on that after submitting my application.

aethelwulffe commented 5 years ago

By figuring out what is wrong with the query? Gotta validate the query (with values for the array variables) to make sure the joins to the facilities table work and all that, then figure out why the whole report is having issues with dates and the like. I imagine the date picker was re-implemented and no-one actually verified the function after that.

Ngai-E commented 5 years ago

Ok I am working on that.

Ngai-E commented 5 years ago

@aethelwulffe finally succeeded to validate the query as you suggested. Also fixed the date ranges issues. I send a pull request, patiently waiting for feedback. Thanks.