Hi David, Read the threads on the topic. thanks for pointing them out.
For safe allow-html="safe" it does not seem to work well with these 2 examples Bold a word: Hi <span style="font-weight: bold;">There</span><br> The converted safe html is Hi There<br> Color a word: Hi <span style="color: rgb(255, 0, 0);">There</span><br> The converted safe html is Hi There<br> Color, bold are relatively common operations. I don't know how to improve the allow-html options to make them better, but not being able to put simple text and rich text features is a pain. I realize that selectively turning off html is not a good idea. Security is as good as the weakest link in chain.. So an opening anywhere would be bad. There are a few places in ofbiz where allow-html="any" is specified. You may want to remove those as well to keep security barrier high. Harmeet On Mon, Jun 22, 2009 at 2:56 PM, David E Jones <[email protected]> wrote: > > Thank you for writing this up Harmeet. Before getting into the details it > is important that you understand that you are proposing to change something, > or in fact basically get rid of something, that was discussed in a lot of > detail and that has seen effort from quite a few people. In general when > recommending to change something it's a good idea to understand where that > came from and why things are done the way they are. Based on what you've > written in this and other messages it does not seem that you have done this > research. I might be wrong, and if so I apologize, but in order to continue > with a discussion like this it is important that you know the background and > understand the problem and the reasons for the solutions. > > There is a good thread that discusses most of this, and it is available > here: > > http://www.nabble.com/Security-Issues-td21622188.html > > > On Jun 22, 2009, at 11:31 AM, Harmeet Bedi wrote: > > A issue(https://issues.apache.org/jira/browse/OFBIZ-2645) is that default >> value of allow-html is very restrictive. >> This is as per start with most constrained best practice in security >> >> Default is allow-html="none" >> It not only does not allow html but it also does not allow simple text >> like "Tom's age is likely > Paul's age". '>' breaks. >> > > I agree this could be improved, but the trick is how do we tell the > difference? One interesting page that shows just how difficult it is to > recognize HTML is this one which is oriented around finding hacks related to > this very problem in various browsers: > > http://ha.ckers.org/xss.html > > What you have mentioned is definitely a weakness in the current code, which > is admittedly fairly simple, and an opportunity to improve that code. I > don't think it would be a good idea to change the overall approach and throw > away the attempt to filter incoming text, especially text coming in from > untrusted sources. > > Around the time this was written I asked for recommendations from anyone on > the mailing list that might be more familiar with this topic, especially as > it relates to what a browser will or will not interpret as markup. My > limited knowledge on the topic is that you pretty much have to have > something that looks like a tag, in other words starts with a less than or > open angle bracket and has some sort of word after it, in order for the > browser to consider it markup and try to interpret it. > > One thing that might work is to allow a > if there is no <. I'm not sure > about the other way around because if there is otherwise valid HTML a > browser might not care about a missing > to close a tag. It may also be > valid to allow a < if there is a space after it, but I'm not sure about that > either. > > Please keep in mind that the reason for this is to not allow abuse of a > production system. If that is not important to you or to your clients then > you can certainly disable it globally. However, I'm guessing that's not what > you would want to do and the best approach for all of this is to simply > improve the code so that it allows as much as possible while still > protecting against the security threat it is meant to. > > allow-html="safe" also does not seem to work well. It does not allow well >> formed html. >> > > Could you be more specific? Depending on what you're seeing there may very > well be a bug with the safe HTML code, or just a need to change the > antisamy-esapi.xml configuration file for it. In any case, it would be good > to find out more about what you mean by this because it sounds like a bug. > > Here is a proposal: >> HTML and descriptive text with characters like '>' should be allowable >> whenever there is a description text input by user. >> Change services that deal with description/comments/reason/noteInfo fields >> to have allow-html="any" >> >> >> An example of this is 'updateWorkEffortNote' service. it could change from >> >> <service name="updateWorkEffortNote" engine="simple" >> >> location="component://workeffort/script/org/ofbiz/workeffort/workeffort/WorkEffortSimpleServices.xml" >> invoke="updateWorkEffortNote" auth="true"> >> <description>Update a WorkEffort Note</description> >> <attribute name="workEffortId" type="String" mode="IN" >> optional="false"/> >> <attribute name="noteId" type="String" mode="IN" optional="false"/> >> <attribute name="internalNote" type="String" mode="IN" >> optional="false"/> >> <attribute name="noteInfo" type="String" mode="IN" optional="true"/> >> </service> >> >> >> to >> >> <service name="updateWorkEffortNote" engine="simple" >> >> location="component://workeffort/script/org/ofbiz/workeffort/workeffort/WorkEffortSimpleServices.xml" >> invoke="updateWorkEffortNote" auth="true"> >> <description>Update a WorkEffort Note</description> >> <attribute name="workEffortId" type="String" mode="IN" >> optional="false"/> >> <attribute name="noteId" type="String" mode="IN" optional="false"/> >> <attribute name="internalNote" type="String" mode="IN" >> optional="false"/> >> <attribute name="noteInfo" type="String" mode="IN" optional="true" >> allow-html="any"/> >> </service> >> >> >> if this seems acceptable, i can send patches with services for review and >> commits. >> > > I mentioned some things related to this in my comments above, but in > general no, my opinion is that we should not open up this security hole in > so many places by default. I would be very interested to hear what others > have to say and how others would like to see this working, but it may be > that they are not talking so much because this has already been discussed. > > In general it is not safe to assume that output filtering will always take > care of this problem, and so when ever text comes from an un-trusted source, > or a potentially un-trusted source, we should filter the input to avoid the > problem right there. > > I guess this goes back to what I said at the beginning of this message. It > would be very valuable to put this in the context of what has already been > researched and discussed. What you have written here would be a lot more > meaningful and a lot easier to discuss if you referred back to the original > discussion and decisions that lead to the functionality you are looking at. > What I mean by that is that anything that has been discussed and decided on > can certainly be changed, but not without referring back to the original > discussion and decisions. > > -David > > > >
