Skip to content
This repository has been archived by the owner on Mar 10, 2024. It is now read-only.

Mysql injections #26

Closed
justinfarmer14 opened this issue Sep 4, 2017 · 8 comments
Closed

Mysql injections #26

justinfarmer14 opened this issue Sep 4, 2017 · 8 comments

Comments

@justinfarmer14
Copy link
Contributor

i was notified by a member on discord mysql injection is possible assign to Anthony for security flaw

@phillf
Copy link
Contributor

phillf commented Sep 4, 2017

@justinfarmer14 Do we have details of where and when this is possible?

@isoadam
Copy link

isoadam commented Sep 4, 2017

There are many vulnerable queries littered throughout the source

As an example:
https://github.com/StormlightTech/openCAD-php/blob/development-stable/actions/api.php#L1015-L1023

@phillf
Copy link
Contributor

phillf commented Sep 4, 2017

Would it be worth writing a SQL library to handle queries now or going through and patching all of these and saving that library for later?

@phillf phillf self-assigned this Sep 4, 2017
@isoadam
Copy link

isoadam commented Sep 4, 2017

I would personally suggest overhauling database interaction. Maybe consider an ORM?

I think reviewing interaction with the database will be important for securing the performance and scalability of the project.

My current understanding is that you authenticate with the database on nearly every request - this is not the greatest idea.
Consider complex application logic which may require multiple database transactions - authenticating with the database multiple times will slow the application times of the application, increasing response times - not the greatest idea for realtime applications and data.

I have not worked with PHP for a couple of years now - but it will be important to ensure that only one connection/authentication is made to the database per request.

If you wish to continue using the mysqli driver, consider exclusively using prepared statements, this should greatly reduce the chance of SQL injection vulnerable processes being present in the application.

@phillf
Copy link
Contributor

phillf commented Sep 4, 2017

(Leaving this for reference)
http://propelorm.org/

@isoadam
Copy link

isoadam commented Sep 4, 2017

Looks good!

@ItsAGeekThing
Copy link
Member

I'm going to do a full security audit in the next few days

@ItsAGeekThing
Copy link
Member

ItsAGeekThing commented Sep 16, 2017

We should consider having the integration of an ORM as a separate report.

@ItsAGeekThing ItsAGeekThing modified the milestones: 1.0 RC1, 1.0 RC2 Sep 23, 2017
@phillf phillf assigned TylerrOC and unassigned phillf Sep 25, 2017
@phillf phillf closed this as completed Jan 30, 2020
Ramius1701 pushed a commit to Ramius1701/openCAD-php that referenced this issue Aug 8, 2022
…PLETON/opencad-php:feature/OCPHP-184-ability-to-hide-leo-tools-from-fire-ems to release/development

Approved by farmer 11:37 3/30/18

* commit '32bd70d73814392b0c3266b569275e130b2848f5':
  Fire/EMS can no longer see/use LEO tools
  #OCPHP-185 #comment Tweaked the documentation to be more verbose. Also added a "Feature Settings" section in oc-config.sample.php and moved all feature settings under that heading. #close"
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants