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. allow-html="safe" also does not seem to work well. It does not allow well formed html. 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. Harmeet |
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 |
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 <[hidden email]> 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 > > > > |
On Jun 28, 2009, at 2:53 PM, Harmeet Bedi wrote: > 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. The original configuration I put in SVN is the one from the ESAPI project based on what is allowed on slashdot. We can certainly change it... The configuration is in the "antisamy-esapi.xml" file. I just did a small update to allow the span tag, though I think it doesn't allow the style attribute. We may want to allow that too... any thoughts anyone? On a side note, bold is easier to do with the <b> tag, or with a <h?> tag. I don't know if there is a good alternative for color though... That said, we should see what the WYSIWIG HTML textarea editor in OFBiz now generates and make sure that is supported, and of course if there are any that others are using that generate HTML that OFBiz is not accepting as "safe" we should change it for those too. In short, I don't think the antisamy-esapi.xml file has been touched at all since I put the original slashdot based one in there, and it certainly should be touched for these things! > 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. Do you have any specific instances of this you have noticed? I've tried to keep an eye on commits to look for service attributes that are set this way but that may not come from a reliable/safe source... but I know I miss a lot of stuff and honestly don't read every line of every commit. -David > > On Mon, Jun 22, 2009 at 2:56 PM, David E Jones <[hidden email]> 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 >> >> >> >> |
[Harmeet]
> There are a few places in ofbiz where allow-html="any" is specified. [David] Do you have any specific instances of this you have noticed? doing search on allow-html="any" gave me the following services sendInvoicePerEmail createEmailContent updateEmailContent persistDataResourceAndData createCommunicationEventInterface sendMail sendMailFromUrl sendMailFromScreen prepareNotificationInterface sendNotificationInterface Entire security is as good as weakest link in chain. so you may want to remove them. Harmeet |
Was suggesting remove allow-html="any" from service, not service itself.
----- Original Message ----- From: "Harmeet Bedi" <[hidden email]> To: [hidden email] Sent: Monday, June 29, 2009 3:43:01 PM GMT -05:00 US/Canada Eastern Subject: Re: proposal related to allow-html defaults [Harmeet] > There are a few places in ofbiz where allow-html="any" is specified. [David] Do you have any specific instances of this you have noticed? doing search on allow-html="any" gave me the following services sendInvoicePerEmail createEmailContent updateEmailContent persistDataResourceAndData createCommunicationEventInterface sendMail sendMailFromUrl sendMailFromScreen prepareNotificationInterface sendNotificationInterface Entire security is as good as weakest link in chain. so you may want to remove them. Harmeet |
In reply to this post by Harmeet Bedi
some of your remarks are now set to 'safe',
thanks for reporting regards, Hans On Mon, 2009-06-29 at 15:43 -0400, Harmeet Bedi wrote: > [Harmeet] > > There are a few places in ofbiz where allow-html="any" is specified. > > [David] > Do you have any specific instances of this you have noticed? > > doing search on allow-html="any" gave me the following services > > sendInvoicePerEmail > createEmailContent > updateEmailContent > persistDataResourceAndData > createCommunicationEventInterface > sendMail > sendMailFromUrl > sendMailFromScreen > prepareNotificationInterface > sendNotificationInterface > > Entire security is as good as weakest link in chain. so you may want to remove them. > > Harmeet > Antwebsystems.com: Quality OFBiz services for competitive rates |
Free forum by Nabble | Edit this page |