Menu
▾
▴
Re: [WebERP-developers] New branch working for DB_query parameterisation work
|
From: Phil D. <ph...@lo...> - 2014-03-28 07:05:11
|
Jo, I can find it now - but have you written your ideal new function - named other than DB_query - and where the parameter type is given explicitly rather than inferred? Would be interested to see that... perhaps since we are doing this we should jump right on in? I am also rethinking the TableView class idea to eliminate the <tr><td></td></tr> all over the place - and perhaps maybe conversion to <div> with optional classes? that might more easily accommodate phones etc. We are of course looking it almost a complete re-write here! Let's go :-) Phil Phil Daintree Logic Works Ltd - +64 (0)275 567890 http://www.logicworks.co.nz On 28/03/14 19:07, icedlava wrote: > 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 > ------------------------------------------------------------------------------ > _______________________________________________ > Web-erp-developers mailing list > Web...@li... > https://lists.sourceforge.net/lists/listinfo/web-erp-developers > |
