Single-click to set the end date for issues

OpenEMR Development
1

mdsupport wrote on Thursday, June 09, 2011:

Requesting review / comments on small update to stats_full.php.

2

acmoore wrote on Thursday, June 09, 2011:

I haven’t been paying too closely to the development process recently, so I’m wondering if it’s our policy to use placeholders in SQL statements these days instead of directly adding CGI variables into SQL like this:

 sqlQuery( "UPDATE lists ".
       "SET enddate=NOW() ".
       "WHERE id=".$_POST["endissue"]
 );

Is there a concern of SQL injection problems here?

Thanks,
-Andy

3

yehster wrote on Thursday, June 09, 2011:

Aggree with acmoore.  This would be preferable.

sqlQuery( "UPDATE lists ". "SET enddate=NOW() ". "WHERE id=?",
array(.$_POST["endissue"] )
);
4

mdsupport wrote on Thursday, June 09, 2011:

Makes sense.  Thanks Andy.

5

bradymiller wrote on Friday, June 10, 2011:

Hi,

Hi,

In php coding, two things need to be done to the $_REQUEST variables before using in a sql statement.
1. Deal with magic quotes
2. Escape for sql

The answer is very easy if the script is using the new security method, which is described here:
http://www.openmedsoftware.org/wiki/Active_Projects#PLAN

The scripts that use this new security model are obvious (contain the $fake_register_globals=false; and $sanitize_all_escapes=true; at the top of the script). Several modules have been converted (listed at bottom of wiki section) and I write all my new scripts with this mechanism (I try to get others to also use this with new scripts, but has been difficult to enforce with the large amount of code coming in lately). A cool thing about this mechanism is that it deals with the magic_quotes automatically, so all you need to do is number 2) above. So, in these script, binding/placemarking in the sql call is all you need to do.

However, in the other scripts that do not use the new security model(such as this one), need to consider the magic quotes also. We have functions in formdata.inc.php that help deal with these scripts. We used to use the formData function, however, I’d suggest using the strip_escape_custom() function to deal with magic quotes along with binding/placemarking to deal with the sql escaping. This scheme will also be easier to convert the script to the new security model in the future.

-brady

6

bradymiller wrote on Saturday, June 11, 2011:

Hi,

Just reviewed/tested this. The main problem is that this script has undergone significant changes over last month which aren’t included in your code. I merged your code into the most recent codebase (three conflicts were fixed), but there are still bugs in it. Here’s the merged code:
http://github.com/bradymiller/openemr/commits/single-click-end-issue_1
Once you fix the bugs, then will test it out again.
thanks,
-brady