Menu
▾
▴
Re: [WebERP-developers] New branch working for DB_query parameterisation work
|
From: icedlava <ice...@gm...> - 2014-03-28 07:34:00
|
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 |
