Menu
▾
▴
Re: [WebERP-developers] New branch working for DB_query parameterisation work
|
From: icedlava <ice...@gm...> - 2014-03-28 07:43:06
|
Phil, Can I commit TWIG to the new branch and a simple integration - existing code will work as normal but it would provide somewhere to add twig templates and themes for proof of concept or comparison? If it is not required, or wanted, it could be removed later. What do you think? 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 |
