Menu
▾
▴
Re: [WebERP-developers] New branch working for DB_query parameterisation work
|
From: icedlava <ice...@gm...> - 2014-03-28 06:08:03
|
Hi Phil, > As you say, my revised DB_query() just escapes the parameters as they > were previously - just that we have now moved the escaping to the > place > where it needs to happen rather than blanket escaping every > $_POST/$_GET > - so we are equally secure as we were ... unless I am wrong and those > variables which should not be escaped are not escape Yes, exactly - we are equally secure as we were before for database transactions. As you say, the whole reason that this topic came up was due to non-contextual escaping of variables and the issues that has and is continuing to cause in the code/database. We have the opportunity now to fix it, but also improve the security of webERP as we fix the original variable problem, which is what I've tried to do with the version I committed. Thanks for your understanding! Cheers, On 28 Mar 2014, at 16:27, Phil Daintree wrote: > Hi Jo, > > As you say, my revised DB_query() just escapes the parameters as they > were previously - just that we have now moved the escaping to the > place > where it needs to happen rather than blanket escaping every > $_POST/$_GET > - so we are equally secure as we were ... unless I am wrong and those > variables which should not be escaped are not escaped. > > That said, I will look into the full prepare execute method in more > detail. Of course with my blinkered vision what I did is more readable > :-) I am persuaded by the argument to do things the most secure way > though given we are into a major re-write and I think this may be the > full prepare/execute method you first suggested. > > Phil > > Phil Daintree > Logic Works Ltd - +64 (0)275 567890 > http://www.logicworks.co.nz > > On 28/03/14 16:45, icedlava wrote: >> Hi Phil, >> >> For reference here on the list, I have committed the code as >> requested. >> You have updated the original function I committed which changes and >> I >> did make a point to you in email that these changes will not work. >> >> As per your request I've forwarded your response to the mailing list >> but >> posting here for completeness in this thread. >> >> Note that the latest changes will not work because it is not the fact >> that the query is prepared, it is the fact the query is paramaterized >> that makes it secure. >> Parameterized queries, done in the way I did it work because the >> query >> string and the variables are sent separately to the database server >> and >> only there are put together. >> >> The changes you made are no different to the original DB_query that >> mysql_db_escapes the data - except there is more code and separated >> parameters. The key issue: >> >> In your new code: >> $result=mysqli_query($Conn, $SQLString); is the problem. No different >> to >> prior old DB_query. >> >> The query is already 'put together' with query and variables before >> it >> goes to the server. That is the insecure nature of it and why I >> changed >> it. >> >> Appreciate your work and thought on it so far. >> >> Cheers, >> >> >> >> >> On 25 Mar 2014, at 17:10, Phil Daintree wrote: >> >>> I've made up a new branch so we can mess with this a bit >>> >>> svn checkout --username=XXXXX >>> svn+ssh://XX...@sv.../p/web-erp/code/branches/working >>> yourworkingcopy >>> >>> where XXXXX is your sourceforge login. >>> >>> Jo, if you commit the code you want for the new DB_query - and I >>> propose >>> we should keep it the same name - but with a new optional parameter >>> for >>> the array of parameters. >>> >>> >>> -- >>> Phil >>> >>> Phil Daintree >>> Logic Works Ltd - +64 (0)275 567890 >>> http://www.logicworks.co.nz >>> >>> >>> ------------------------------------------------------------------------------ >>> Learn Graph Databases - Download FREE O'Reilly Book >>> "Graph Databases" is the definitive new guide to graph databases and >>> their >>> applications. Written by three acclaimed leaders in the field, >>> this first edition is now available. Download your free book today! >>> http://p.sf.net/sfu/13534_NeoTech >>> _______________________________________________ >>> Web-erp-developers mailing list >>> Web...@li... >>> https://lists.sourceforge.net/lists/listinfo/web-erp-developers >> ------------------------------------------------------------------------------ >> _______________________________________________ >> Web-erp-developers mailing list >> Web...@li... >> https://lists.sourceforge.net/lists/listinfo/web-erp-developers >> > > > ------------------------------------------------------------------------------ > _______________________________________________ > Web-erp-developers mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/web-erp-developers |
