SQL Injection vulnerability

[ra1nb0rn]

Hi, I believe to have found an SQL Injection vulnerability that exists once the datatable for Physical Exam Diagnoses has been installed. Because I don't know how you prefer to handle security issues like these, I have decided not to post any more information yet. I would appreciate it if you could get back to me whether I should describe the details here or somewhere else.

Regards

[muarachmann]

Hi @DustinBorn thanks for raising such. Here would be a perfect place to describe the issue

[ra1nb0rn]

Hi @muarachmann Thank you for your quick response. I've written a small report that should describe the vulnerability and possible fixes in good detail. If requested I can also give you a proof of concept.

SQL Injection in LibreHealth EHR via Web Request in interface/forms/physical_exam/edit_diagnoses.php

Description

The vulnerable SQL statement can be found in lines 62-65 in the source file interface/forms/physical_exam/edit_diagnoses.php:

$dres = sqlStatement(
 "SELECT * FROM form_physical_exam_diagnoses WHERE " .
 "line_id = '$line_id' ORDER BY ordering, diagnosis"
);

As this statement shows, the table form_physical_exam_diagnoses has to exist for the SQLi to work properly. The variable that is used for the injection is $line_id, which is originally set equal to the GET or POST parameter (or Cookie) lineid in line 12 of the same file:

$line_id = $_REQUEST['lineid'];

As $line_id does not get modified before the SQL query in lines 62-65, the parameter can be exploited to achieve an SQL Injection.

Suggested Fixes / Solutions

SQL Injection vulnerabilities can be fixed in different ways. For a descriptive list see https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet. As an example, we can fix this vulnerability by using Prepared Statements. As such, we can modify the SQL query in lines 62-65 to prevent SQL Injection like so:

$dres = sqlStatement(
 "SELECT * FROM form_physical_exam_diagnoses WHERE " .
 "line_id = ? ORDER BY ordering, diagnosis", array($line_id)
);

[muarachmann]

@DustinBorn you are right and thanks for pointing this out. We have similar cases hanging out there. Are you interested in fixing this issue. If yes you can go ahead. We would very much appreciate your help. Otherwise our team will handle this 😃

[ra1nb0rn]

Yeah, sure @muarachmann. Give me a day or two. I'll fix it and then issue a Pull Request.