Menu
▾
▴
Re: [WebERP-developers] New branch working for DB_query parameterisation work
|
From: Jon R <jr...@gm...> - 2014-03-28 07:38:09
|
If we use the Views classes TWIG can be implemented in a theme. I will post a proof of concept to the github site in the next day or so. ~serakfalcon *Sent from my iLaptop* On Fri, Mar 28, 2014 at 3:33 PM, 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 > |
