ariesbeck wrote on Wednesday, April 10, 2013:
I installed OpenEMR and have been doing some minor customization work for a client of ours. I decided to look into the login process to get a general sense of the security of the system, figuring the login would be the most reviewed and secured area, and I have to say I was quite shocked. Once I got past the fact that the latest version still uses a salt-less hashing algorithm I noticed that all of the SQL queries in auth.inc are susceptible to SQL injection.
Am I wrong about this? As far as I can see the call to authNewSession uses unaltered POST variables as its arguments. Those arguments are directly passed into an SQL call such as “update users set password=’$new_upgrade_pass’ where username = ‘$user’”.
I hope I am wrong about this. A lot of new features are being added everyday to OpenEMR and I am truly grateful for all it provides. But if SQL Injection holes exist, and especially during login, then the focus really needs to be security before all else. We are talking about patient data here, arguably some of the most sensitive data existent in the commercial world.
The combination of the auth module having sql injection vulnerabilities AND the fact that hashing is done on the client side basically means that if the sql injection allowed a user to see the hashed password of any user, all they would have to do would be send that hash back to the login script to gain access to that account. Hashing on the client side is completely and totally pointless. One of the huge benefits of hashing is to never store the raw value that would need to be passed to the form to authenticate. This way if a user somehow retrieved the hashed password of a user from a misguided sql statement they would not be able to simply use that password to login.
It seems someone could do a regular expression query to identify any calls to sqlStatement which contain a $ in the first argument.
I can’t dedicate enough time at this exact moment but I may become more involved and try to help patch these vulnerabilities because they are very very serious. I will need to come up with quicker solutions and probably limit access to my install to specific IP addresses and client side certificates until these issues can be addressed. I cant believe a hacker hasn’t completely dumped every OpenEMR system deployed to date.
Aaron