Menu
▾
▴
Re: [WebERP-developers] Replicating slashes problem - escaping special chars for database insertion or HTML
|
From: Tim S. <tim...@gm...> - 2013-11-27 16:49:26
|
Hi Jo, I guess what I don't understand is what changing the code will
give us? At the moment we have a solution that works (albeit the
occasional change to remove the slashes, but most of these have been
done) and has passed the security audits we have had done. I am not
wedded to the current solution, it's more a case of "if it ain't broke
why fix it?".
Tim
On 27 November 2013 16:43, icedlava <ice...@gm...> wrote:
> Tim,
> using stripslahes with our current code covers up the problem, but doesn't fix the root issue.
>
> I'm happy to do the work to fix it if it will give us a clean solution.
>
> On 28 Nov 2013, at 3:11, Tim Schofield wrote:
>
>> Hi Jo, yes I did try it with purchase orders and see the problem.
>> Doesn't changing line 741 to
>>
>> <td><input type="text" name="ItemDescription' . $POLine->LineNo . '"
>> size="30" value="' . stripslashes($POLine->ItemDescription) . '"
>> /></td>
>>
>> fix the problem?
>>
>> Likewise in DeliveryDetails.php changing line 998 to
>>
>>
>> <td><input type="text" autofocus="autofocus"
>> required="required" size="42" maxlength="40" name="DeliverTo" value="'
>> . stripslashes($_SESSION['Items'.$identifier]->DeliverTo) . '"
>> title="' . _('Enter the name of the customer to deliver this order
>> to') . '" /></td>
>>
>>
>> I may be missing something. There are actually only a few scripts
>> where the POST data isn't immediately posted to the DB.
>>
>> thanks
>> Tim
>>
>>
>>
>> On 27 November 2013 16:31, icedlava <ice...@gm...> wrote:
>>> Hi Tim,
>>>
>>> Yes I agree there are some instances that we dealt with specifically but the problem remains and is more than just slashes replicating and is across the codebase in many fields.
>>>
>>> session.inc assumes all post, get and session vars are for the database and escape them via the db_escape function (and actually turns everything to entities as well for the db!). When the vars are not for the database at first instance, it causes problems. Try your test with Purchase order not sales order which we did fix in most parts. Here we also 'fill in a form' and the item is posted causing special char issues.They also come out on documentation that goes to customers.
>>>
>>> There are many other parts of the code base where special chars in general are not processed properly or also entities are not processed properly or doubly processed and saved in the database.
>>>
>>> *** we end up with entities in the database (which should not happen) causing display issues when output. ***
>>>
>>> *** we end up with special char issues like the multiplying slashes with each db save ***
>>>
>>> As you mentioned, the code in session.inc was written many years ago. It still uses magic quote calls (deprecated way back in php 5.3 2009) known not to be good solution for security although of course we try to support still all those on old php.
>>>
>>> It does not mean we cannot use best practice for handling data going into the database or out to display.
>>>
>>> It would be wonderful to have a system wide consistent solution to fix the entity issues in our database, that also will fix the escaping of data meant for display. It is better than just patching instances, and stripslashes() itself is not a full solution. I know it might be some work but happy to do it if we come up with agreed method - it would certainly fix a lot of customer complaint with display issues for me too :)
>>>
>>> I will think on some possible alternatives but i'm sure there are those out there with good solutions already.
>>>
>>> Thanks for all the useful comments!
>>>
>>>
>>>
>>> On 28 Nov 2013, at 0:57, Tim Schofield wrote:
>>>
>>>> Hi Jo,
>>>>
>>>> I meant I thought we had dealt with most instances. For instance
>>>> creating a sales order for smith's crisps does not generate this
>>>> error. I think the overwhelming majority of webERP scripts are of the
>>>> type: "Fill in a form, submit it and update the database". Fixing this
>>>> *seems* to be just a case of using stripslashes() on the item
>>>> description in the PO_Items.php script.
>>>>
>>>> This code in session.inc came about after a security review by Steve
>>>> Lord many years ago, and I would be very nervous about taking it out.
>>>>
>>>> Thanks
>>>> Tim
>>>>
>>>> On 27 November 2013 12:30, icedlava <ice...@gm...> wrote:
>>>>> Hi Tim,
>>>>>
>>>>>> thought we had caught and dealt with these
>>>>>
>>>>> We caught an instance but I knew at the time it required a broader fix
>>>>> but was unable to identify best way ahead for it.
>>>>>
>>>>> I have raised the issue some time ago with Phil but not had time until
>>>>> now to look at the code and had a better look today.
>>>>>
>>>>>> Can you give some
>>>>>> examples of where it is still happening?
>>>>>
>>>>> Perhaps you can try replicate - it could be text with special char in
>>>>> any text field that is submitted in a form, a displayed value or input
>>>>> value that has some e.g. post var, get var or session var submitted to
>>>>> the database (some specific instances in the code have been addressed
>>>>> and fixed like you mentioned).
>>>>>
>>>>> I will try and provide an example:
>>>>>
>>>>> 1. Create an item with an item description such as "Smith's Chips"
>>>>> 2. Place a purchase order and select the item for your order.
>>>>> 3. At the PO_items.php function, with the line item for the order
>>>>> selected, click on Update Order lines and watch the description. You
>>>>> should get more slashes each time you click the Update Order lines
>>>>> button.
>>>>>
>>>>> I think, that we cannot assume in the sessions.inc file that we are
>>>>> going to add/update a post or get or session var to a database at first
>>>>> instance - it could be displayed only, it could be used in an input post
>>>>> field etc
>>>>>
>>>>> - when we save to the database call the relevant function e.g.
>>>>> mysqli_real_escape_string on non-html entity string (in weberp this is
>>>>> the DB_escape_string function but should be nonentity string e.g. use
>>>>> htmlspecialchars_decode not htmlspecialchars).
>>>>>
>>>>> - when we display in HTML use htmlspecialchar call to encode to entities
>>>>> where relevant.
>>>>>
>>>>> Anyway this is just some issue i came across a while ago and now
>>>>> revisiting in hope we can squash it for good - maybe someone has a
>>>>> solution already.
>>>>>
>>>>> Cheers!
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 27 Nov 2013, at 18:57, Tim Schofield wrote:
>>>>>
>>>>>> Hi Jo,
>>>>>>
>>>>>> I thought we had caught and dealt with these. Can you give some
>>>>>> examples of where it is still happening?
>>>>>>
>>>>>> Thanks
>>>>>> Tim
>>>>>>
>>>>>> On 27 November 2013 06:57, iced lava <ice...@gm...> wrote:
>>>>>>> Currently we have sessions.inc processing all session, post and get
>>>>>>> vars in
>>>>>>> the same way.
>>>>>>>
>>>>>>> We make a big assumption here with DB_escape_string at line 59 and 66
>>>>>>> (for
>>>>>>> single var and slightly different for arrays):
>>>>>>>
>>>>>>> $_POST[$PostVariableName] = DB_escape_string($PostVariableValue);
>>>>>>>
>>>>>>> This assumes that we know what a function will do with the
>>>>>>> post/get/sessions
>>>>>>> vars and that they are going to be inserted/updated to a database.
>>>>>>> They are
>>>>>>> passed to the DB_escape_string function which returns (for mysqli):
>>>>>>>
>>>>>>> mysqli_real_escape_string($db, htmlspecialchars($String,
>>>>>>> ENT_COMPAT,'utf-8',
>>>>>>> false));
>>>>>>>
>>>>>>> This is causing exponential slash addition to variables which are not
>>>>>>> entered to a database but instead posted to a HTML field value. Each
>>>>>>> time
>>>>>>> the field is updated the value is saved in the database with extra
>>>>>>> slashes
>>>>>>> when escaping a special char like a single quote.
>>>>>>>
>>>>>>> If we do actually need to do this variable processing in sessions.inc
>>>>>>> then
>>>>>>> perhaps we could decide what is more common displaying the var or
>>>>>>> insert/updating to a db.
>>>>>>>
>>>>>>> If we chose for example that displaying the var on the page is more
>>>>>>> common
>>>>>>> then we should change sessions inc for example on this line:
>>>>>>>
>>>>>>> $_POST[$PostVariableName] = DB_escape_string($PostVariableValue)
>>>>>>>
>>>>>>> to
>>>>>>> $_POST[$PostVariableName]= htmlspecialchars($PostArrayValue,
>>>>>>> ENT_QUOTES,'UTF-8');
>>>>>>>
>>>>>>> and for this case in DB_escape_string from:
>>>>>>>
>>>>>>> return mysqli_real_escape_string($db, htmlspecialchars($String,
>>>>>>> ENT_COMPAT,'utf-8', false));
>>>>>>> to
>>>>>>>
>>>>>>> return mysqli_real_escape_string($db,
>>>>>>> htmlspecialchars_decode($String,
>>>>>>> ENT_COMPAT));
>>>>>>>
>>>>>>>
>>>>>>> Then we need to call DB_escape_string on vars before we send to any
>>>>>>> insert
>>>>>>> or edit for database.
>>>>>>>
>>>>>>> On the other hand - we need to do the opposite if we assume it is
>>>>>>> more
>>>>>>> likely to post to the database.
>>>>>>>
>>>>>>> Perhaps there is another 3rd way to handle it?
>>>>>>>
>>>>>>> Thanks to all in advance for any feedback!!
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ------------------------------------------------------------------------------
>>>>>>> Rapidly troubleshoot problems before they affect your business. Most
>>>>>>> IT
>>>>>>> organizations don't have a clear picture of how application
>>>>>>> performance
>>>>>>> affects their revenue. With AppDynamics, you get 100% visibility into
>>>>>>> your
>>>>>>> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of
>>>>>>> AppDynamics
>>>>>>> Pro!
>>>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
>>>>>>> _______________________________________________
>>>>>>> Web-erp-developers mailing list
>>>>>>> Web...@li...
>>>>>>> https://lists.sourceforge.net/lists/listinfo/web-erp-developers
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Course View Towers,
>>>>>> Plot 21 Yusuf Lule Road,
>>>>>> Kampala
>>>>>> T +256 (0) 312 314 418
>>>>>> M +256 (0) 752 963 325
>>>>>> www.weberpafrica.com
>>>>>> Twitter: @TimSchofield2
>>>>>> Blog: http://weberpafrica.blogspot.co.uk/
>>>>>>
>>>>>> ------------------------------------------------------------------------------
>>>>>> Rapidly troubleshoot problems before they affect your business. Most
>>>>>> IT
>>>>>> organizations don't have a clear picture of how application
>>>>>> performance
>>>>>> affects their revenue. With AppDynamics, you get 100% visibility into
>>>>>> your
>>>>>> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of
>>>>>> AppDynamics Pro!
>>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
>>>>>> _______________________________________________
>>>>>> Web-erp-developers mailing list
>>>>>> Web...@li...
>>>>>> https://lists.sourceforge.net/lists/listinfo/web-erp-developers
>>>>>
>>>>> ------------------------------------------------------------------------------
>>>>> Rapidly troubleshoot problems before they affect your business. Most IT
>>>>> organizations don't have a clear picture of how application performance
>>>>> affects their revenue. With AppDynamics, you get 100% visibility into your
>>>>> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
>>>>> _______________________________________________
>>>>> Web-erp-developers mailing list
>>>>> Web...@li...
>>>>> https://lists.sourceforge.net/lists/listinfo/web-erp-developers
>>>>
>>>>
>>>>
>>>> --
>>>> Course View Towers,
>>>> Plot 21 Yusuf Lule Road,
>>>> Kampala
>>>> T +256 (0) 312 314 418
>>>> M +256 (0) 752 963 325
>>>> www.weberpafrica.com
>>>> Twitter: @TimSchofield2
>>>> Blog: http://weberpafrica.blogspot.co.uk/
>>>>
>>>> ------------------------------------------------------------------------------
>>>> Rapidly troubleshoot problems before they affect your business. Most IT
>>>> organizations don't have a clear picture of how application performance
>>>> affects their revenue. With AppDynamics, you get 100% visibility into your
>>>> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
>>>> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
>>>> _______________________________________________
>>>> Web-erp-developers mailing list
>>>> Web...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/web-erp-developers
>>>
>>> ------------------------------------------------------------------------------
>>> Rapidly troubleshoot problems before they affect your business. Most IT
>>> organizations don't have a clear picture of how application performance
>>> affects their revenue. With AppDynamics, you get 100% visibility into your
>>> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
>>> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
>>> _______________________________________________
>>> Web-erp-developers mailing list
>>> Web...@li...
>>> https://lists.sourceforge.net/lists/listinfo/web-erp-developers
>>>
>>
>>
>>
>> --
>> Course View Towers,
>> Plot 21 Yusuf Lule Road,
>> Kampala
>> T +256 (0) 312 314 418
>> M +256 (0) 752 963 325
>> www.weberpafrica.com
>> Twitter: @TimSchofield2
>> Blog: http://weberpafrica.blogspot.co.uk/
>>
>> ------------------------------------------------------------------------------
>> Rapidly troubleshoot problems before they affect your business. Most IT
>> organizations don't have a clear picture of how application performance
>> affects their revenue. With AppDynamics, you get 100% visibility into your
>> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
>> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Web-erp-developers mailing list
>> Web...@li...
>> https://lists.sourceforge.net/lists/listinfo/web-erp-developers
>
> ------------------------------------------------------------------------------
> Rapidly troubleshoot problems before they affect your business. Most IT
> organizations don't have a clear picture of how application performance
> affects their revenue. With AppDynamics, you get 100% visibility into your
> Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
> http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
> _______________________________________________
> Web-erp-developers mailing list
> Web...@li...
> https://lists.sourceforge.net/lists/listinfo/web-erp-developers
>
--
Course View Towers,
Plot 21 Yusuf Lule Road,
Kampala
T +256 (0) 312 314 418
M +256 (0) 752 963 325
www.weberpafrica.com
Twitter: @TimSchofield2
Blog: http://weberpafrica.blogspot.co.uk/
|
