Skip to main content

SQL Injection security flaw in OpenEMR medical records system.

I recently examined a popular open source medical records system named OpenEMR. A quick review of the app uncovered a SQL Injection vulnerability in the application, that would allow an attacker to execute their own SQL commands against the system. The attack is relatively textbook and its detection and exploitation are outlined below. Firstly, a description of the product:
Profile: OpenEMR is a medical practice management software which also supports Electronic Medical Records (EMR). It is ONC Complete Ambulatory EHR certified and it features fully integrated electronic medical records, practice management for a medical practice, scheduling and electronic billing.

The server side is written in PHP and can be employed in conjunction with a LAMP "stack", though any operating systems with PHP-support are also supported.
In the US, it has been estimated that there are more than 5,000 installations of OpenEMR in physician offices and other small healthcare facilities serving more than 30 million patients. Internationally, it has been estimated that OpenEMR is installed in over 15,000 healthcare facilities, translating into more than 45,000 practitioners using the system which are serving greater than 90 million patients.

Source: Wikipedia:
Affected versions: OpenEMR 4.1.2 Patch 5 (and likely previous patches & releases)
Fix in: OpenEMR 4.1.2 Patch 6 

As usual I reviewed the system as a user, browsing features and recording my actions in my intercepting proxy (BurpSuite). This gave me a good idea of the default system features and usage model. Combined with review through the online documentation, I gained a broad idea of how the system is used and its features or ‘claims’.

The latest/patched code was relatively well protected against SQL Injection, with widespread use of prepared statements, a good defence against 1st order SQL Injection. But, I noticed a few queries were not parameterised. While this is not necessarily a problem, if its possible to include custom inputs into the query, then vulnerabilities can creep in.

In this case, the affected query was a delete for ‘Patient Disclosures’. When the user opts to delete a Disclosure record via the user interface the system runs this query, inserting the record identifier sent via the browser.

Unfortunately, the Open EMR system does not filter out inappropriate characters for these requests, meaning SQL can be written unmodified into the request. As long as the SQL, when combined with the remainder of the query is valid syntactically, the query is then executed. If code had restricted the input to be, for example positive integers, then this vulnerability would be largely mitigated.

You can see the vulnerable code here:

File: openemr-4.1.2/library/
function deleteDisclosure($deletelid)


       $sql="delete from extended_log where id='$deletelid'";

       $ret = sqlInsertClean_audit($sql);


As you can see the ID string is just included directly into the string used for the query.

As a proof of concept, I wrote a simple SQL extract that when injected produces a valid but nefarious query. In this case, the query deletes all Patient Disclosures.

The malicious Request URL might look like this (the malicious characters in red):


The active code inserted is:
' OR '1'='1

This generates a SQL query like this:
delete from extended_log where id='5' OR '1'='1'

The addition ensures every item in the table is deleted. Not only those with an id of 5. Other injections are of course possible, this one was chosen because its a simple demonstration of SQL Injection. Typically an attacker would try to extract user credentials, or confidential information  - in this case possibly patient medical records.

One positive aspect of the flaw is that it is not pre-auth. So the attack only works when the attacker/exploit code has access to a valid logged-in session. This makes it slightly harder to exploit, but not overly so as an attacker can use methods such as Cross Site Request Forgery to initiate ‘blind’ attacks from another browser tab. But in summary, if OpenEMR is deployed only on a local network this issue is not severe.

Note: I reported this issue in a process of responsible disclosure on a 30 day embargo. (That expired 5 days before a patch was released and 9 days before this post.  

The patch was released on the 8th June 2014 and is meant to address this issue and others. (Look for the fixes from Brady Miller to I have not tested this fix.


Popular posts from this blog

Why you might need testers

I remember teaching my son to ride his bike. No, Strike that, Helping him to learn to ride his bike. It’s that way round – if we are honest – he was changing his brain so it could adapt to the mechanism and behaviour of the bike. I was just holding the bike, pushing and showering him with praise and tips.
If he fell, I didn’t and couldn’t change the way he was riding the bike. I suggested things, rubbed his sore knee and pointed out that he had just cycled more in that last attempt – than he had ever managed before - Son this is working, you’re getting it.
I had help of course, Gravity being one. When he lost balance, it hurt. Not a lot, but enough for his brain to get the feedback it needed to rewire a few neurons. If the mistakes were subtler, advice might help – try going faster – that will make the bike less wobbly. The excitement of going faster and better helped rewire a few more neurons.
When we have this sort of immediate feedback we learn quicker, we improve our game. When the f…

Thank you for finding the bug I missed.

Thank you to the colleague/customer/product owner, who found the bug I missed. That oversight, was (at least in part) my mistake. I've been thinking about what happened and what that means to me and my team.

I'm happy you told me about the issue you found, because you...

1) Opened my eyes to a situation I'd never have thought to investigate.

2) Gave me another item for my checklist of things to check in future.

3) Made me remember, that we are never done testing.

4) Are never sure if the application 'works' well enough.

5) Reminded me to explore more and build less.

6) To request that we may wish to assign more time to finding these issues.

7) Let me experience the hindsight bias, so that the edge-case now seems obvious!

Being a square keeps you from going around in circles.

After a weary few hours sorting through, re-running and manually double checking the "automated test" results, the team decide they need to "run the tests again!", that's a problem to the team. Why? because they are too slow. The 'test' runs take too long and they won't have the results until tomorrow.
How does our team intend to fix the problem? ... make the tests run faster. Maybe use a new framework, get better hardware or some other cool trick. The team get busy, update the test tools and soon find them selves in a similar position. Now of course they need to rewrite them in language X or using a new [A-Z]+DD methodology. I can't believe you are still using technology Z , Luddites!
Updating your tooling, and using a methodology appropriate to your context makes sense and should be factored into your workflow and estimates. But the above approach to solving the problem, starts with the wrong problem. As such, its not likely to find the right ans…