Closed dshorthouse closed 7 years ago
Good catch. We have been going through the code patching SQLi vulnerabilities, but we still come across some issues in the older code sections.
It has been a few years that we have been talking about switching over to PDO, or some other db abstraction solution. It remains on the todo list, but as you state, it would be a major undertaking.
From what I can see in setProj($proj) in https://github.com/Symbiota/Symbiota/blob/master/classes/InventoryProjectManager.php#L266 and how it gets called from https://github.com/Symbiota/Symbiota/blob/master/projects/index.php#L20, this looks susceptible to SQL Injection via the proj query param. I suspect there will be many other instances where a user's raw query is being passed through to a SQL WHERE statement; I wasn't really looking for this but stumbled across it.
I suggest switching to PDO and using prepared statements, but this would/could be a massive undertaking. However, far better to invest in this overhaul than to risk user data disappearing through nefarious activity.