SQL Injection - Stored Procedures

[SLSCoder]

My reporting app dynamically creates a rather complex query including fields and a filter derived from client side user input.
The query would include joins from multiple tables.
It pulls records, 20 at a time so paging  will be used: My Post About Paging

I'm concerned about 2 things: speed and SQL injection.

Since the app will use paging I'd like to create a stored procedure that can be called repeatedly.
It would be created dynamically, based  on user input.
The fields and the filter in the stored procedure would be created based on user input but would not change once the report is created.
I'd rather NOT make the parameters of the filter MySql 'parameters' because they won't change (and I want it to be fast) but I'm concerned about potential SQL injection.

The only 'parameters' that will change are the RecordIDs in the range I need for the next page.

Is there any way I can create a stored procedure and make the filter (from user input) NOT require parameters that must be included every time the procedure is called?
That is, is there a way to write the filter from user input into the stored procedure without the filter values being variables and not have to worry about SQL injection?

 

 

[requinix]

Do you still feel the same way if I tell you that MySQL caches queries in memory? That if you prepare a statement one time, preparing it again later will be easier? That to an extent, queries themselves are cached too?

In other words, that kind of performance isn't something you need to worry about right now, and the level of performance you should care more about is of the queries themselves.

[SLSCoder]

Thanks for responding.

I'm not sure I understand. Are saying just use PDO, and make the user inputs PDO labels and let the code recreate it every time it's used?
The query will change (slightly) each time it's used due to the paging. I think I"m going to use LIMIT and not an ID range for paging.

[requinix]

It's not about PDO but about MySQL. If you tell MySQL that you want to run a certain query, it will remember that for a while in case you want to run it again. And not in a literal sense of "here is the query as a string and does it match a previous query" but in a more sophisticated way that doesn't care much about LIMITs or the exact values submitted with a prepared statement.

So you're probably good to go without trying to do anything special. Remember: databases like MySQL are built by very smart people who know a lot about what they're doing and how the system needs to be used, so just use it normally and see how it goes.

[gizmola]

Quote

My reporting app dynamically creates a rather complex query including fields and a filter derived from client side user input.

You have described pretty much every database application that has paginated lists of data.  Rarely do you see these features built with sprocs.

To add briefly to requinix's comments:

I believe the default mysql engine now is InnoDB.  Make sure you are using the innodb engine for your tables.   You want them to be InnoDB tables.  InnoDB has a true data cache component, so not only will the query be cached, but the actual result set will be cached.  You can configure the amount of server memory allocated to the cache.  

Ideally you can achieve a high percentage of cache hits in a properly resourced server, so that data is repeatedly coming from cache rather than having to be pulled from persistent storage, although with the proliferation of high performance SSD's, reading from disk is less an issue than it used to be, but is still something that you should aspire to, even if it's just extending the lifetime of your SSD's.

I have nothing against sprocs, but it sounds like a lot of overkill for this project for very little gain.  

 

[SLSCoder]

Thanks. The querys would a) require multiple sql statements and b) be called repeatedly for months and even years.

gizmola: it is InnoDB. quote: "Rarely do you see these features built with sprocs." Why not? They could call the sproc over and over with 2 params, LastID and Limit.

So far, I have heard a whole lot about why I shouldn't create stored procedures with PDO (or at all) but nothing about how to do it.
Simple EXAMPLE: the user wants a report on a summary of the answers to questions where the rated entities (not the surveyors) are: Project Managers (job_types:21) of vendors(organizations) that have conducted projects where the project type (project_types:17) is Health Care.
This would be a classic report for a user to monitor the performance of vendors' project managers doing health care; in my opinion, not too complicated.
The USER determines what they want the report to show them via user inputs in the report builder that I'm trying to develop.

I want to create a query that can be called repeatedly for years that matches the criteria the user selected.
I'd really rather not have to store all the field names and values and add them as parameters every time the app calls the report because they won't change.
The problem is that user input fields may attempt sql injection and I need to prevent that.

If I write a CREATE STORED PROCEDURE (programmatically) that includes: " WHERE projects.ProjectTypeID = :ProjectTypeID"
and then add a parameter to the PDO (not the sproc): $ppd->bindValue(":ProjectTypeID", "17", PDO::PARAM_INT);  
I just want PDO to plunk the value 17 into the PDO label :ProjectTypeID

If it would do that I'd have my stored procedure.
From there all I'd have to do would be set the Next ID and the Limit in parameters ie: "CALL sprcReport37(0, 20)"
This seems very simple to me.

Again, the procedure would include multiple sql statements.
When I try to do this using PDO the execute returns true and there are no errors but PDO will not create the stored procedure.

requinix: "do anything special" Is writing a stored proc that special? It's not hard to do if I could just get past the sql injection.
If I did I could recall it over and over and over with the sql: "CALL sprcReport37(0, 20)". That's a lot easier for me.
Why is writing a stored procedure programmatically such a big deal?

Can I programmatically create a stored procedure having multiple sql statements in PHP using PDO for MySql?
OR  - Can I programmatically create a stored procedure using anything that will protect against user input sql injection without making all the input values stored procedure parameters?

 

[requinix]

You seem really fixated on this "I have to create a stored procedure in my code" thing when you don't have to do that.

According to the code you posted, which is a really important point to make so if the code you posted is not what you actually want to do then you need to say something right now (and I'm about 99% sure it is not what you want),

DROP PROCEDURE IF EXISTS test1; 
DELIMITER || 
CREATE PROCEDURE test1(IN LastID INT(5)) 
BEGIN 
SELECT person.RecordID AS PersonID, organization.Name FROM person INNER JOIN organization ON person.lnk_organization = organization.RecordID LIMIT 10;
SELECT person.RecordID AS PersonID, organization.Name, organization.City, organization.State, organization.Zip FROM person INNER JOIN organization ON person.lnk_organization = organization.RecordID LIMIT 25; 
END || 
DELIMITER ;

I'll say again what I've said before: there is nothing in that query which means you have to create it in your PHP code at the point when you want to call it. There is no information in there that depends on something only the code knows.

So do me a favor and try something, okay?
Create your "test1" stored procedure manually and remove the stuff about creating it from your code. Now try calling it. You can supply whatever "LastID" parameter you feel like. SQL injection won't be a thing because "LastID' is typed as an integer and SQL injection isn't possible with an integer.

Now, you say you want to do something with variable limits. Okay. Have you tried doing that? Because recent MySQL will just let you do that: make the two limit numbers be parameters and specify those variables in the LIMIT clauses.

Worst case, your desired stored procedure does not match your posted stored procedure, and the desired stored procedure actually has some aspect that cannot be parameterized. I don't know what it would be.
If that was the case then you still (probably) wouldn't create a stored procedure at runtime because that is tied to the answer of a particular question: would your code, the code that hypothetically creates this stored procedure, ever call it multiple times? I don't mean "yes, it would call the procedure every time the code runs". I mean once that procedure was created, would the code that is currently running want to use it multiple times, and would completely separate code anywhere else running shortly after that code also want to use it? I can't imagine why your answer would be anything other than "no", which means you don't need a stored procedure because you can just run those SELECT statements at the time you want them. There's absolutely zero reason to create a stored procedure if it's only going to be used once.

[SLSCoder]

requinix: Thanks for your response but I can't get you to read my post.
The code is a TEST.
 I am trying to programmatically create a stored procedure that includes values that are derived from client side form fields and prevent sql injection.
My purpose for an sproc is so that my app can call it repeatedly for years after it is created. It is a report created by the user, in the report builder which is what I'm trying to develop.

I don't understand why this has to be so complicated and I can't understand why you have so much of a problem just answering my question:
How can I programmatically create a stored procedure that includes values that are derived from client side form fields and prevent sql injection?

[SLSCoder]

The correct answer to this question is that it cannot be done.
That is, there is no way using PDO or mysqli prepared statements to create stored procedures from client form inputs as parameters and therefore no way to prevent sql injection.

The reason is that prepared statement parameters (PDO or mysqli) cannot be saved as part of a query. The parameters the database, not as part of the sql.

The PHP code to create the prepared statement and if not still cached the MySql work to optimize the prepared statement must be executed every time the prepared statement is used.
If the prepared statement is to be run repeatedly the parameters must be stored initially and then retrieved every time the prepared statement is called.

I think it would be worthwhile to find better ways than prepared statements to prevent sql injection.

[gizmola]

On 9/29/2021 at 9:26 AM, SLSCoder said:

gizmola: it is InnoDB. quote: "Rarely do you see these features built with sprocs." Why not? They could call the sproc over and over with 2 params, LastID and Limit.

First, let me just opine that there are generally accepted reasons to create stored procedures.  Those include 'performance', 'adding business logic', 'doing things that can't easily be done in a single query/ie having procedural logic', 'providing a procedural api that enforces business rules', and in the case of triggers, enforcing complex data integrity, which is often done with triggers, and can't easily or robustly done client-side.

What you have to understand about MySQL, is that it doesn't work the same way that Sybase/MS-SQL Server or Oracle work.  In those DB's, sprocs are cached in global server memory, so they can be shared by connections.  Oracle also has heavy client connection overhead.  MySQL does not work that way.

Quite probably, a normal query will be faster with MySQL in many circumstances, when compared with a sproc, because you have to understand that MySQL sprocs are not available in a shared memory structure like Oracle.  So performance is not one of the advantages of sprocs in MySQL.

The sproc memory exists PER Connection!  So that should give you pause, from a performance standpoint, because each connection will need memory allocation for sprocs, and conversely, the fact that clientA is calling a sproc, does absolutely nothing for clientB.  There has been rumblings that something might be done about this architecture, but as of MySQL 8, as far as I know the per connection sproc cache is still local.

So to be absolutely clear, what happens when you create a connection to MySQL, every time you use a sproc, it gets compiled (if it was not already used), and stored in memory.  There is not pre-compilation performance boost you get from other databases like Oracle.  

Furthermore, PHP is a "shared nothing" environment.  Depending on how you are running PHP, database connections will be created/destroyed frequently, or upon every execution.  The fact that mysql connections are lightweight and performant is one of the reasons it has always been a good partner for PHP data persistence.

Quote

I'm concerned about 2 things: speed and SQL injection.

This was your original concern.  Most of us tried to convince you that you already are covered for those concerns by:

• Disallowing multiple statements in PDO
• Using bind variables
• Using InnoDB with allocation of memory to buffer pools, to maximize cache hit of result set data

PHP does give you a robust and highly capable language to build your reporting tool, and your code can be safe and will be performant against mysql, and sprocs bring nothing to the table that will make that better for you.  

I understand that you have felt frustrated in this conversation, but this is a frequent phenomenon in the tech communities I frequent, when someone comes from a point of view that has predetermined a particular approach is the only way to do it.  People immediately question whether or not, as the old adage goes, this is a "person with a hammer, who sees everything as a nail."  

I think this was a valuable thread that contributed to the community, and I appreciate your perseverance and patience in sticking with it, but I also hope you can see that developers who are donating their time to try and help other developers tend to get a bit irritated when they perceive that someone is telling them "just shutup and answer my question", especially when they aren't convinced that the problem to be solved has been articulated clearly.

With that said, I hope you will continue to find the forum valuable to you now and in the future. 

  

[SLSCoder]

gizmola: Thank you for that response.
I appreciate your information about MySql. I used to develop in VB ASP and then C#.NET using DBase then MS Sql Server so I guess I expected MySql to be more like Sql Server.

The PHP code that creates these queries is over 1,000 lines. It's not so much that the 'query' would be that big, it's just versatile and the code has to cover every possibility.
My reason for wanting to use sprocs in part was so that I don't have to run all that PHP code every time the query is used. Also, it does require more than 1 sql statement and I was hoping to do it all in one db communication.
The queries are derived from user input. My intention was to create the query structure once and then from there all I'd have to do would be call it. That would speed the PHP code and I thought, also the MySql code.

I've learned that I cannot create/store queries from client (I mean the browser) form inputs. From what I understand the only way I can prevent sql injection is with prepared statements (PDO or mysqli).
That means I cannot *save* the queries and then reuse them. I have to rebuild them every time I use them. I have to store the parameters/values and then retrieve them every time I use the queries.
That will *definitely* slow the app down.

I expected to be able to save the queries (multiple sql statements) to something  compiled & optimized by MySql and then be able to just use it repeatedly.
I don't really care about the fact that sproc memory is allocated per connection. Generally, one person creates a report and that will be the only person who uses it.
They will however use it over and over and over, sometimes for years.

I appreciate that the people here do this for free. I've depended on forums like this to help me for as long as they have existed on the Internet.
I've also contributed to them myself quite a bit.
I *am* open to suggestions as to how to accomplish my goals. The only 'answers' I've gotten here involve prepared statements which prevent me from being able to *save* the queries.
That means I have to retrieve the parameter/value pairs and then rebuild the queries in PHP every time I use them. Also there is *some* procedural code I would have liked to run in MySql.

Thank you all for your help.
 

[gizmola]

10 hours ago, SLSCoder said:

 

I've learned that I cannot create/store queries from client (I mean the browser) form inputs. From what I understand the only way I can prevent sql injection is with prepared statements (PDO or mysqli).
That means I cannot *save* the queries and then reuse them. I have to rebuild them every time I use them. I have to store the parameters/values and then retrieve them every time I use the queries.
That will *definitely* slow the app down.
 

In my experience this really isn't the case, again due to caching.  I'm sure you know as well, that the main area of result set optimization is index coverage.  One other thing to know about MySQL with InnoDB is that the data is stored as a "clustered index".  In other words, the data is stored in primary key order, and whenever you read a row where it was retrieved through the use of the primary key, there is no index read cost, and you get the data for the entire row.   This also perhaps helps explain the value of the result set cache, as MySQL can return data from Server memory pages, and will use a number of techniques to optimize the use of cache, rather than reading from disk.  There is some overhead in parsing the queries, but it's less of a concern in MySQL than in other db's.  

 

Quote

I expected to be able to save the queries (multiple sql statements) to something  compiled & optimized by MySql and then be able to just use it repeatedly.
I don't really care about the fact that sproc memory is allocated per connection. Generally, one person creates a report and that will be the only person who uses it.
They will however use it over and over and over, sometimes for years.

I might not have explained this clearly, but again, in MySQL, sprocs are not pre-compiled because someone used one at some point.   They get compiled for each connection, when requested.  After that, the sproc will be reusable in compiled form, for the life of the connection, but as I stated previously, in a PHP application, your connections will be either frequently closed, or closed on every connection.  The details of that, go into how PHP is being run, but in either case, new connections and re-compilation of sprocs will be frequent with MySQL.  The more you have, the more memory will be required to store the compiled sprocs, so if you have a lot of sprocs and a lot of connections, that memory could be significant, but if this is an intranet environment where you don't have a hugh query volume or lots of concurrent requests, it's probably not reason for concern either.  

There is one mechanism that might be of interest in regards to the compilation of sprocs, which is to use a persistent connection:

<?php
$dbh = new PDO('mysql:host=localhost;dbname=test', $user, $pass, array(
    PDO::ATTR_PERSISTENT => true
));

 The PDO::ATTR_PERSISTENT => true setting will trigger, again depending on your web server configuration, a connection pool, if for example you are using php-fpm.  As frequently as you can find information about it, you will also find people telling you not to use it, as it can cause issues like deadlocks, hung connections that lead to timeouts/500 errors in your application, etc, but again in an Intranet environment it might not be a problem.

I have at times in my career made heavy use of sprocs, triggers and views, so I'm not predisposed not to use them, but in my substantial experience with using MySQL, it is rare that I've found they were needed.   Typically, when I have used them, it was to insure some sort of programmatic locking/transaction that I wanted to be guaranteed.  One example that comes to mind is an implementation of a homegrown sequence object with naming and business rules around the range boundaries of the sequence numbers.  It's always good to remember things like triggers and sprocs often will cause serialization and reduce concurrency.

The other thing about sprocs, is that you are only moving code around.  And in the process of moving code to the db, you are adding complexity to your development environment, as you have to make your changes to the database, rather than in your frontend code, when coding/debugging.  You aren't going to have less code.

[Strider64]

On 10/1/2021 at 8:21 AM, SLSCoder said:

The correct answer to this question is that it cannot be done.
That is, there is no way using PDO or mysqli prepared statements to create stored procedures from client form inputs as parameters and therefore no way to prevent sql injection.

The reason is that prepared statement parameters (PDO or mysqli) cannot be saved as part of a query. The parameters the database, not as part of the sql.

The PHP code to create the prepared statement and if not still cached the MySql work to optimize the prepared statement must be executed every time the prepared statement is used.
If the prepared statement is to be run repeatedly the parameters must be stored initially and then retrieved every time the prepared statement is called.

I think it would be worthwhile to find better ways than prepared statements to prevent sql injection.

When I first started coding in PHP there was a person on here (I can't remember his name) that helped me out a lot. I was hung up on securing my code and spent all night writing a script that I thought was "secure" then the next day I posted it here. He replied back to me and the first thing he wrote was to throw that script in the trash. He went on to say people who write security code work in teams and test them out before they are introduced to the wild (internet). There is nothing wrong with prepared statement and unless you are a large corporation the speed of prepared statements is fast enough. Prepared statements are as secure as they possibly can be and nothing is 100 percent secure. So I totally disagree on the use of prepared statement.

Edited October 6, 2021 by Strider64