Menu
▾
▴
Re: [WebERP-developers] New branch working for DB_query parameterisation work
|
From: Phil D. <ph...@lo...> - 2014-03-28 07:46:04
|
Yes please re perfect parameterized query function and maybe AccountGroups.php reworked I think twig undermines the readability. On 28 March 2014 8:33:35 PM NZDT, icedlava <ice...@gm...> wrote: >Hi Phil, > >> 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 > >Did you want me to put it in the new branch? > >> I am also rethinking the TableView class idea to eliminate the >> <tr><td></td></tr> all over the place - > >I would support that - I think we have too much dependence on tables >for >layout generally in the themes and using divs would be better. > >I think we should think very carefully about what classes are used and >needed in the 'table less' themes and try and keep them to a minimum. >CSS allows us to do great things with minimal markup. > >I do believe that tables are still important for data views which is >where their forte in use is. > >> We are of course looking it almost a complete re-write here! > >Sounds like it lol! I'm up for it :) > >We could do with a list of other things to do while we are at it. >I guess TWIG is still out ? It maybe just me but I don't find the >code >easy to look at as it's mixed thoroughly with html output. >I'd still like to separate it but understand this might be going too >far >right now :p > >Cheers, > > > >On 28 Mar 2014, at 17:34, Phil Daintree wrote: > >> 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 >>> >> >> >> >------------------------------------------------------------------------------ >> _______________________________________________ >> 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 Phil Daintree +64(0)275 567890 skype: daintree |
