[jira] [Commented] (OFBIZ-9800) French translation of OFBiz website
[ https://issues.apache.org/jira/browse/OFBIZ-9800?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842331#comment-16842331 ] Jacques Le Roux commented on OFBIZ-9800: Hi Deepak, Yes I think so too. If nobody complains I'll close in a week. > French translation of OFBiz website > --- > > Key: OFBIZ-9800 > URL: https://issues.apache.org/jira/browse/OFBIZ-9800 > Project: OFBiz > Issue Type: Improvement > Components: site >Reporter: Olivier Heintz >Assignee: Deepak Dixit >Priority: Minor > Attachments: website-fr.tar, website-fr.tar, website-fr.tar.gz, > website-fr.tar.gz, website-fr.tar.gz, website-fr.tar.gz, website-fr.tar.gz > > > To evaluate the workload of translate all the ofbiz website page in french, > and so to maintain the translation when there are some modifications, I have > start to translate them. > There are between 10 and 15 page to translate, and translate one is between 1 > and 2 hours. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-4975) updatePortletSeqDragDrop is not working in PortalPage management
[ https://issues.apache.org/jira/browse/OFBIZ-4975?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842328#comment-16842328 ] Jacques Le Roux commented on OFBIZ-4975: Hi Pierre, Olivier, Pierre I saw that you updated the affected versions field with not supported versions (R13,14,15). As this is a bug I suspect we should close it if the trunk is not concerned because it will not be backported in those. If the trunk and supported versions are concerned then why did you not put in? Thanks. > updatePortletSeqDragDrop is not working in PortalPage management > > > Key: OFBIZ-4975 > URL: https://issues.apache.org/jira/browse/OFBIZ-4975 > Project: OFBiz > Issue Type: Bug > Components: myportal >Affects Versions: Release Branch 13.07, Release Branch 14.12, Release > Branch 15.12 >Reporter: Olivier Heintz >Assignee: Erwan de FERRIERES >Priority: Minor > Attachments: OFBIZ-4975.patch > > > When you try to change Portlet order in a PortalPage, most of the time it > does'nt work. > To show why, I start to change in minilang service, calculate attribute by a > set > {code} > > > > > - > - > - > - > - > - > - > - > - > - > + from="newSequenceNo+increase" /> > + > > > > > {code} > the error was to not use a second field, the correct code should be > {code} > > > > > > > >from="newSequenceNo+newValue" /> > > > > > > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-4742) Portlet Widget
[ https://issues.apache.org/jira/browse/OFBIZ-4742?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842330#comment-16842330 ] Jacques Le Roux commented on OFBIZ-4742: Hi Olvier, Should we expect more work on this or rather close? > Portlet Widget > -- > > Key: OFBIZ-4742 > URL: https://issues.apache.org/jira/browse/OFBIZ-4742 > Project: OFBiz > Issue Type: Improvement > Components: framework >Affects Versions: Trunk >Reporter: Olivier Heintz >Priority: Major > Attachments: OFBIZ-4742.patch, > OFBIZ-portletWidget_iconsPurpose_Example_Help.patch > > > Description: Goals of this enhancement are : > - simplification of portlet development by use of portlet Templates > (PortletType), so most of the time, it's sufficient to define a form to have > a portlet > - default value of formName, menuName, ScreenName, ScriptName, Title, ... > (name and location) based on component, subcomponent, webapp of the portlet > - ajax call defined with xml syntax (new show-portlet on form and menu) > - default parameters list in ajax link (show-portlet or on-event-update-area) -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-10969) Unable to create Employments
[ https://issues.apache.org/jira/browse/OFBIZ-10969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842321#comment-16842321 ] Jacques Le Roux commented on OFBIZ-10969: - Hi Olivier, I, for one, am not strictly against. But I don't very like to commit temporary things in order to help a contributor. I already seen such things not ending very well. Of course that could be discussed in dev ML and a community agreement would remove this issue on my side, and would make it more known, not only by you and me. BTW, you say: bq. because without commit I cannot use the current test (Employee Training and Development) on OFBiz demo. Can't you work on your side without commiting in trunk? What does block you? Can't you have several instances with different settings running in parallel on your own machine? BTW as it's a bug this will need to be backported. I guess all supported releases are concerned, right? > Unable to create Employments > > > Key: OFBIZ-10969 > URL: https://issues.apache.org/jira/browse/OFBIZ-10969 > Project: OFBiz > Issue Type: Bug > Components: humanres >Affects Versions: Trunk >Reporter: Arpit Mor >Assignee: Jacques Le Roux >Priority: Major > Attachments: Image1.png > > > Steps to regenerate: > # Login to the URL: > [https://demo-trunk.ofbiz.apache.org/humanres/control/main] > # Click on Employments > # Click on New Employments > # Click on Create > Actual: Error message is displayed. Please refer attachment: Image1 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Closed] (OFBIZ-11041) Incorrect findByCount on DynamicView with groupBy and selected field
[ https://issues.apache.org/jira/browse/OFBIZ-11041?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Nicolas Malin closed OFBIZ-11041. - Resolution: Fixed Fix Version/s: 18.12.01 Upcoming Branch 16.11.06 17.12.01 > Incorrect findByCount on DynamicView with groupBy and selected field > > > Key: OFBIZ-11041 > URL: https://issues.apache.org/jira/browse/OFBIZ-11041 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Nicolas Malin >Assignee: Nicolas Malin >Priority: Minor > Fix For: 17.12.01, 16.11.06, Upcoming Branch, 18.12.01 > > > When you create a DynamicView with group by function and you realize a > delegator.findCountByCondition() with selected fields, the result was > different than count each element returned by delegator.findList(). > In cause : > {code:java} > Index: > framework/entity/src/main/java/org/apache/ofbiz/entity/datasource/GenericDAO.java > @@ -1031,7 +1031,7 @@ > > // GROUP BY clause for view-entity > if (isGroupBy) { > -modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(), > sqlBuffer, " GROUP BY ", ", ", "", false); > + > modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(selectFields), > sqlBuffer, " GROUP BY ", ", ", "", false); > } > {code} > modelViewEntity.colNameString returns all grouped by fields for realize goup > by closure and not only thegrouped by fields present on selected fields. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-11041) Incorrect findByCount on DynamicView with groupBy and selected field
[ https://issues.apache.org/jira/browse/OFBIZ-11041?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842275#comment-16842275 ] Nicolas Malin commented on OFBIZ-11041: --- Fixed at revision: * Trunk: 1859438 * 18.12: 1859439 * 17.12: 1859440 > Incorrect findByCount on DynamicView with groupBy and selected field > > > Key: OFBIZ-11041 > URL: https://issues.apache.org/jira/browse/OFBIZ-11041 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Nicolas Malin >Assignee: Nicolas Malin >Priority: Minor > > When you create a DynamicView with group by function and you realize a > delegator.findCountByCondition() with selected fields, the result was > different than count each element returned by delegator.findList(). > In cause : > {code:java} > Index: > framework/entity/src/main/java/org/apache/ofbiz/entity/datasource/GenericDAO.java > @@ -1031,7 +1031,7 @@ > > // GROUP BY clause for view-entity > if (isGroupBy) { > -modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(), > sqlBuffer, " GROUP BY ", ", ", "", false); > + > modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(selectFields), > sqlBuffer, " GROUP BY ", ", ", "", false); > } > {code} > modelViewEntity.colNameString returns all grouped by fields for realize goup > by closure and not only thegrouped by fields present on selected fields. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (OFBIZ-11041) Incorrect findByCount on DynamicView with groupBy and selected field
Nicolas Malin created OFBIZ-11041: - Summary: Incorrect findByCount on DynamicView with groupBy and selected field Key: OFBIZ-11041 URL: https://issues.apache.org/jira/browse/OFBIZ-11041 Project: OFBiz Issue Type: Bug Components: framework Affects Versions: Trunk Reporter: Nicolas Malin Assignee: Nicolas Malin When you create a DynamicView with group by function and you realize a delegator.findCountByCondition() with selected fields, the result was different than count each element returned by delegator.findList(). In cause : {code:java} Index: framework/entity/src/main/java/org/apache/ofbiz/entity/datasource/GenericDAO.java @@ -1031,7 +1031,7 @@ // GROUP BY clause for view-entity if (isGroupBy) { -modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(), sqlBuffer, " GROUP BY ", ", ", "", false); + modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(selectFields), sqlBuffer, " GROUP BY ", ", ", "", false); } {code} modelViewEntity.colNameString returns all grouped by fields for realize goup by closure and not only thegrouped by fields present on selected fields. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (OFBIZ-11040) Manage EECAs on delegator.removeBy
[ https://issues.apache.org/jira/browse/OFBIZ-11040?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Nicolas Malin updated OFBIZ-11040: -- Attachment: OFBIZ-11040.patch > Manage EECAs on delegator.removeBy > -- > > Key: OFBIZ-11040 > URL: https://issues.apache.org/jira/browse/OFBIZ-11040 > Project: OFBiz > Issue Type: Improvement > Components: framework >Affects Versions: Trunk >Reporter: Nicolas Malin >Assignee: Nicolas Malin >Priority: Minor > Attachments: OFBIZ-11040.patch > > > Currently, when you delete some entities through removeByAnd or > removeByCondition, eeca aren't enable and the remove is quite as regard > implemented rules. > With > {code:java} > event="return"> > > > {code} > And > {code:java} > delegator.removeByAnd('GoodIdentification', [productId: 'WG-']) > {code} > The service indexProduct wasn't call for the productId WG- > To solve this situation, the idea would be delegator.removeValue for each > element to delete when an eeca is present otherwise call the standard > helper.removeByCondition. > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (OFBIZ-11040) Manage EECAs on delegator.removeBy
[ https://issues.apache.org/jira/browse/OFBIZ-11040?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Nicolas Malin updated OFBIZ-11040: -- Description: Currently, when you delete some entities through removeByAnd or removeByCondition, eeca aren't enable and the remove is quite as regard implemented rules. With {code:java} {code} And {code:java} delegator.removeByAnd('GoodIdentification', [productId: 'WG-']) {code} The service indexProduct wasn't call for the productId WG- To solve this situation, the idea would be delegator.removeValue for each element to delete when an eeca is present otherwise call the standard helper.removeByCondition. This patch [^OFBIZ-11040.patch] provided by [~mleila] was: Currently, when you delete some entities through removeByAnd or removeByCondition, eeca aren't enable and the remove is quite as regard implemented rules. With {code:java} {code} And {code:java} delegator.removeByAnd('GoodIdentification', [productId: 'WG-']) {code} The service indexProduct wasn't call for the productId WG- To solve this situation, the idea would be delegator.removeValue for each element to delete when an eeca is present otherwise call the standard helper.removeByCondition. > Manage EECAs on delegator.removeBy > -- > > Key: OFBIZ-11040 > URL: https://issues.apache.org/jira/browse/OFBIZ-11040 > Project: OFBiz > Issue Type: Improvement > Components: framework >Affects Versions: Trunk >Reporter: Nicolas Malin >Assignee: Nicolas Malin >Priority: Minor > Attachments: OFBIZ-11040.patch > > > Currently, when you delete some entities through removeByAnd or > removeByCondition, eeca aren't enable and the remove is quite as regard > implemented rules. > With > {code:java} > event="return"> > > > {code} > And > {code:java} > delegator.removeByAnd('GoodIdentification', [productId: 'WG-']) > {code} > The service indexProduct wasn't call for the productId WG- > To solve this situation, the idea would be delegator.removeValue for each > element to delete when an eeca is present otherwise call the standard > helper.removeByCondition. > This patch [^OFBIZ-11040.patch] provided by [~mleila] -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (OFBIZ-11040) Manage EECAs on delegator.removeBy
[ https://issues.apache.org/jira/browse/OFBIZ-11040?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Nicolas Malin updated OFBIZ-11040: -- Description: Currently, when you delete some entities through removeByAnd or removeByCondition, eeca aren't enable and the remove is quite as regard implemented rules. With {code:java} {code} And {code:java} delegator.removeByAnd('GoodIdentification', [productId: 'WG-']) {code} The service indexProduct wasn't call for the productId WG- To solve this situation, the idea would be delegator.removeValue for each element to delete when an eeca is present otherwise call the standard helper.removeByCondition. was: Currently, when you delete some entities through removeByAnd or removeByCondition, eeca aren't enable and the remove is quite as regard implemented rules. With {code:java} {code} And {code:java} delegator.removeByAnd("GoodIdentification", [productId: 'WG-') {code} The service indexProduct wasn't call for the productId WG- To solve this situation, the idea is call removeValue for each element hen an eeca is present otherwise call the standard helper.removeByCondition. > Manage EECAs on delegator.removeBy > -- > > Key: OFBIZ-11040 > URL: https://issues.apache.org/jira/browse/OFBIZ-11040 > Project: OFBiz > Issue Type: Improvement > Components: framework >Affects Versions: Trunk >Reporter: Nicolas Malin >Assignee: Nicolas Malin >Priority: Minor > > Currently, when you delete some entities through removeByAnd or > removeByCondition, eeca aren't enable and the remove is quite as regard > implemented rules. > With > {code:java} > event="return"> > > > {code} > And > {code:java} > delegator.removeByAnd('GoodIdentification', [productId: 'WG-']) > {code} > The service indexProduct wasn't call for the productId WG- > To solve this situation, the idea would be delegator.removeValue for each > element to delete when an eeca is present otherwise call the standard > helper.removeByCondition. > > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (OFBIZ-11040) Manage EECAs on delegator.removeBy
Nicolas Malin created OFBIZ-11040: - Summary: Manage EECAs on delegator.removeBy Key: OFBIZ-11040 URL: https://issues.apache.org/jira/browse/OFBIZ-11040 Project: OFBiz Issue Type: Improvement Components: framework Affects Versions: Trunk Reporter: Nicolas Malin Assignee: Nicolas Malin Currently, when you delete some entities through removeByAnd or removeByCondition, eeca aren't enable and the remove is quite as regard implemented rules. With {code:java} {code} And {code:java} delegator.removeByAnd("GoodIdentification", [productId: 'WG-') {code} The service indexProduct wasn't call for the productId WG- To solve this situation, the idea is call removeValue for each element hen an eeca is present otherwise call the standard helper.removeByCondition. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-10969) Unable to create Employments
[ https://issues.apache.org/jira/browse/OFBIZ-10969?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842168#comment-16842168 ] Olivier Heintz commented on OFBIZ-10969: If possible not waiting, because without commit I cannot use the current test (Employee Training and Development) on OFBiz demo. I prefer working on the same time on documentation and tests realization Recruitment is the last of the HR process, and it seem it will need some other corrections :( > Unable to create Employments > > > Key: OFBIZ-10969 > URL: https://issues.apache.org/jira/browse/OFBIZ-10969 > Project: OFBiz > Issue Type: Bug > Components: humanres >Affects Versions: Trunk >Reporter: Arpit Mor >Assignee: Jacques Le Roux >Priority: Major > Attachments: Image1.png > > > Steps to regenerate: > # Login to the URL: > [https://demo-trunk.ofbiz.apache.org/humanres/control/main] > # Click on Employments > # Click on New Employments > # Click on Create > Actual: Error message is displayed. Please refer attachment: Image1 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Comment Edited] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842010#comment-16842010 ] Jacques Le Roux edited comment on OFBIZ-5254 at 5/17/19 11:09 AM: -- [^OFBIZ-5254.patch] is a new patch from today, the one I want to commit. I have only a concern. I wanted to use labels instead of English text for the warning messages. But I then cross issues with UtilCodecTests. So the patch contains labels that will maybe not used, hence not committed. I'll check if I can do something about that before committing. I have tested some (simple) scenarios and it should be OK. Anyway that can be easily extended with CustomSafePolicy class. was (Author: jacques.le.roux): [^OFBIZ-5254.patch] is a new patch from today, the one I want to commit. I have only a concern. I wanted to use labels instead of English text for the warning messages. But I then cross issues with UtilCodecTests. So the patch contains labels that will maybe not used, hence not committed. I'll check if I can do something about that before committing. I have tested some (simple) scenarios and it should be OK. Anyway that can be easily extended with CustomSafePolicy class. > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if
[jira] [Updated] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux updated OFBIZ-5254: --- Attachment: (was: OFBIZ-5254.patch) > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years (see OFBIZ-3135) and should really be replaced with an > official, up to date version since that issue was fixed upstream years ago. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux updated OFBIZ-5254: --- Attachment: OFBIZ-5254.patch > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years (see OFBIZ-3135) and should really be replaced with an > official, up to date version since that issue was fixed upstream years ago. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux updated OFBIZ-5254: --- Attachment: (was: OFBIZ-5254.patch) > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years (see OFBIZ-3135) and should really be replaced with an > official, up to date version since that issue was fixed upstream years ago. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux updated OFBIZ-5254: --- Attachment: OFBIZ-5254.patch > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, OFBIZ-5254.patch, > UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years (see OFBIZ-3135) and should really be replaced with an > official, up to date version since that issue was fixed upstream years ago. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux updated OFBIZ-5254: --- Attachment: (was: OFBIZ-5254.patch) > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years (see OFBIZ-3135) and should really be replaced with an > official, up to date version since that issue was fixed upstream years ago. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Comment Edited] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842010#comment-16842010 ] Jacques Le Roux edited comment on OFBIZ-5254 at 5/17/19 10:59 AM: -- [^OFBIZ-5254.patch] is a new patch from today, the one I want to commit. I have only a concern. I wanted to use labels instead of English text for the warning messages. But I then cross issues with UtilCodecTests. So the patch contains labels that will maybe not used, hence not committed. I'll check if I can do something about that before committing. I have tested some (simple) scenarios and it should be OK. Anyway that can be easily extended with CustomSafePolicy class. was (Author: jacques.le.roux): [^OFBIZ-5254.patch] is a new patch from today, the one I want to commit. I have only a concern. I wanted to use labels instead of English text for the warning messages. But I then cross issues with UtilCodecTests. So the patch contains labels that will maybe not used, hence not committed. I'll check if I can do something about that before committing. I have tested some (simple) scenarios and it should be OK. Anyway that can be easily extended with CustomSafePolicy class. > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if
[jira] [Updated] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux updated OFBIZ-5254: --- Attachment: OFBIZ-5254.patch > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, OFBIZ-5254.patch, > UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years (see OFBIZ-3135) and should really be replaced with an > official, up to date version since that issue was fixed upstream years ago. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842089#comment-16842089 ] Jacques Le Roux commented on OFBIZ-5254: When I will get chances,I will also digg into these too (again more as references to myself): https://www.owasp.org/index.php/Web_Application_Firewall. Sounds interesting when you use HTTPD https://www.owasp.org/index.php/OWASP_Java_Encoder_Project > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years
[jira] [Commented] (OFBIZ-7963) Create a Gradle Sonar task
[ https://issues.apache.org/jira/browse/OFBIZ-7963?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842070#comment-16842070 ] Pierre Smits commented on OFBIZ-7963: - I hope to provide a patch for review soon. Just waiting on feedback from an involved party from SonarCloud on approach. > Create a Gradle Sonar task > -- > > Key: OFBIZ-7963 > URL: https://issues.apache.org/jira/browse/OFBIZ-7963 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Jacques Le Roux >Assignee: Pierre Smits >Priority: Minor > > As ever the devil is in details. There is no longer a Sonar plugin available > https://docs.gradle.org/current/userguide/sonar_plugin.htmland we should > rather refer to > http://docs.sonarqube.org/display/SCAN/Analyzing+with+SonarQube+Scanner+for+Gradle > Anyway the most important part is to revivify INFRA-3590 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Issue Comment Deleted] (OFBIZ-10054) Product content management screen doesn't validate trusted users' input
[ https://issues.apache.org/jira/browse/OFBIZ-10054?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux updated OFBIZ-10054: Comment: was deleted (was: About my comment above bq. I should note here though that currently the AntiSamy API is not used in OFBiz. This is something that still need to be clarified with the authors of OFBIZ-10187. Maybe it was easier for them to adapt from XML to Java... Before asking them I remembered that the AntiSamy API has not been updated since 2013, so should be considered as somehow deprecated (it's a century in term of security). ) > Product content management screen doesn't validate trusted users' input > --- > > Key: OFBIZ-10054 > URL: https://issues.apache.org/jira/browse/OFBIZ-10054 > Project: OFBiz > Issue Type: Bug > Components: product >Affects Versions: Trunk, Release Branch 16.11 >Reporter: Jacopo Cappellato >Assignee: Jacques Le Roux >Priority: Major > Fix For: 17.12.01, 16.11.06, 18.12.01 > > > Steps to recreate: > 1) go to (authenticate with admin/ofbiz): > https://localhost:8443/catalog/control/EditProductContent?productId=WG- > 2) set the content of the field labeled "Large Image" to: > non_existent.foo onerror=alert(Hi!); > 3) visit the url: > https://localhost:8443/ecommerce/control/product?product_id=WG- > A popup message will appear with the "Hi!". > Thanks to Loris Nardo for the report. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842065#comment-16842065 ] Jacques Le Roux commented on OFBIZ-5254: I did not dig into it, but it's interesting to let a link about that here: https://android.googlesource.com/platform/packages/apps/UnifiedEmail/+/ec0fa48/src/com/android/mail/utils/HtmlSanitizer.java > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years (see OFBIZ-3135) and should really be replaced with an
[jira] [Commented] (OFBIZ-7963) Create a Gradle Sonar task
[ https://issues.apache.org/jira/browse/OFBIZ-7963?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842066#comment-16842066 ] Pierre Smits commented on OFBIZ-7963: - In order to have code analysis on OFBiz artefacts through available ASF services (CI and external SonarCloud.io) we need to have the build.gradle file adjusted. > Create a Gradle Sonar task > -- > > Key: OFBIZ-7963 > URL: https://issues.apache.org/jira/browse/OFBIZ-7963 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Jacques Le Roux >Assignee: Pierre Smits >Priority: Minor > > As ever the devil is in details. There is no longer a Sonar plugin available > https://docs.gradle.org/current/userguide/sonar_plugin.htmland we should > rather refer to > http://docs.sonarqube.org/display/SCAN/Analyzing+with+SonarQube+Scanner+for+Gradle > Anyway the most important part is to revivify INFRA-3590 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842021#comment-16842021 ] Jacques Le Roux commented on OFBIZ-5254: [^UtilCodec.java] would be the version using labels (more for me to have it secured somewhere) > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years (see OFBIZ-3135) and should really be replaced with an > official, up to date version since that issue was fixed upstream years ago. -- This message was
[jira] [Updated] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux updated OFBIZ-5254: --- Attachment: UtilCodec.java > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch, UtilCodec.java > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years (see OFBIZ-3135) and should really be replaced with an > official, up to date version since that issue was fixed upstream years ago. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842010#comment-16842010 ] Jacques Le Roux commented on OFBIZ-5254: [^OFBIZ-5254.patch] is a new patch from today, the one I want to commit. I have only a concern. I wanted to use labels instead of English text for the warning messages. But I then cross issues with UtilCodecTests. So the patch contains labels that will maybe not used, hence not committed. I'll check if I can do something about that before committing. I have tested some (simple) scenarios and it should be OK. Anyway that can be easily extended with CustomSafePolicy class. > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version
[jira] [Updated] (OFBIZ-5254) Services allow arbitrary HTML for parameters with allow-html set to "safe"
[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jacques Le Roux updated OFBIZ-5254: --- Attachment: OFBIZ-5254.patch > Services allow arbitrary HTML for parameters with allow-html set to "safe" > -- > > Key: OFBIZ-5254 > URL: https://issues.apache.org/jira/browse/OFBIZ-5254 > Project: OFBiz > Issue Type: Bug > Components: framework >Affects Versions: Trunk >Reporter: Christoph Neuroth >Assignee: Jacques Le Roux >Priority: Critical > Labels: security > Attachments: OFBIZ-5254.patch, OFBIZ-5254.patch > > > For any given service with allow-html=safe parameters, the parameter data is > not properly validated. See Model.Service.java:588: > {code} > > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > {code} > Looking at that method: > {code} > public static String checkStringForHtmlSafeOnly(String valueName, String > value, List errorMessageList) { > ValidationErrorList vel = new ValidationErrorList(); > value = defaultWebValidator.getValidSafeHTML(valueName, value, > Integer.MAX_VALUE, true, vel); > errorMessageList.addAll(UtilGenerics.checkList(vel.errors(), > String.class)); > return value; > } > {code} > you can see that it expects the defaultWebValidator.getValidSafeHTML would > add all validation errors to the given ValidationErrorList, but if you look > at the implementation of ESAPI that is not the case. First, consider the > overloaded getValidSafeHTML that takes the ValidationErrorList: > {code}public String getValidSafeHTML(String context, String input, > int maxLength, boolean allowNull, ValidationErrorList errors) throws > IntrusionException { > try { > return getValidSafeHTML(context, input, maxLength, > allowNull); > } catch (ValidationException e) { > errors.addError(context, e); > } > return input; > } > {code} > Then, step into that method to see that ValidationExceptions are only thrown > for things like exceeding the maximum length - not for policy violations that > can be "cleaned", such as tags that are not allowed by the policy: > {code} > AntiSamy as = new AntiSamy(); > CleanResults test = as.scan(input, antiSamyPolicy); > List errors = test.getErrorMessages(); > if ( errors.size() > 0 ) { > // just create new exception to get it logged > and intrusion detected > new ValidationException( "Invalid HTML input: > context=" + context, "Invalid HTML input: context=" + context + ", errors=" + > errors, context ); > } > {code} > I guess that is an expected, although maybe not clearly documented behavior > of ESAPI: Non-cleanable violations throw the exception and therefore will > fail the ofbiz service, while non-allowed tags are cleaned. However, if you > consider ModelService:588 and following lines again: > {code} > StringUtil.checkStringForHtmlSafeOnly(modelParam.name, value, > errorMessageList); > //(...) > if (errorMessageList.size() > 0) { > throw new ServiceValidationException(errorMessageList, this, > mode); > } > {code} > the cleaned return value is ignored. Therefore, you will see an > "IntrusionDetection" in the logs, giving you a false sense of security but > the unfiltered HTML will still go into the service. So, if you want the > service to fail if non-allowed HTML is encountered, you should use > isValidSafeHTML instead. If you want the incoming HTML to be filtered, you > should use the return value of getValidSafeHTML. > Some additional notes on this: > * When changing this, it should be properly documented as users may well be > relying on this behavior - for example, we send full HTML mails to our > customers for their ecommerce purchases and require HTML to go through - so > maybe for services like the communicationEvents allowing only safe HTML might > not be desired. > * The ESAPI code samples above are from version 1.4.4. I was really surprised > to find a JAR that is not only outdated, but patched and built by a third > party, without even indicating that in the filename in OfBiz trunk. This has > been there for years (see OFBIZ-3135) and should really be replaced with an > official, up to date version since that issue was fixed upstream years ago. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-10991) Have a SalesChannel dimension
[ https://issues.apache.org/jira/browse/OFBIZ-10991?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16841945#comment-16841945 ] Pierre Smits commented on OFBIZ-10991: -- Thanks Jacques. I will reach out to the JIRA admins. > Have a SalesChannel dimension > - > > Key: OFBIZ-10991 > URL: https://issues.apache.org/jira/browse/OFBIZ-10991 > Project: OFBiz > Issue Type: Improvement > Components: bi >Affects Versions: Trunk, Release Branch 17.12, Release Branch 18.12 >Reporter: Pierre Smits >Assignee: Pierre Smits >Priority: Major > Labels: SalesChannel, SalesChannelDimension, birt, dimension, dwh > > The component would benefit from a sales channel dimension for future fact > tables and star schema view entities -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Comment Edited] (OFBIZ-10990) Improve the SalesInvoiceItemStarSchema
[ https://issues.apache.org/jira/browse/OFBIZ-10990?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16829983#comment-16829983 ] Jacques Le Roux edited comment on OFBIZ-10990 at 5/17/19 6:25 AM: -- No worries, Swapnil, I value each and every input. Could you share your thoughts on [\[DISCUSSION\] bi/birt component integration|https://lists.apache.org/thread.html/7f7db5645bba8b9de3a0189356300afd0d7651646671ad782b0a557f@%3Cdev.ofbiz.apache.org%3E] ? was (Author: pfm.smits): No worries, Swapnil, I value each and every input. Could you share your thoughts on [[DISCUSSION] bi/birt component integration|https://lists.apache.org/thread.html/7f7db5645bba8b9de3a0189356300afd0d7651646671ad782b0a557f@%3Cdev.ofbiz.apache.org%3E] ? > Improve the SalesInvoiceItemStarSchema > -- > > Key: OFBIZ-10990 > URL: https://issues.apache.org/jira/browse/OFBIZ-10990 > Project: OFBiz > Issue Type: Improvement > Components: bi >Affects Versions: Trunk, Release Branch 17.12, Release Branch 18.12 >Reporter: Pierre Smits >Priority: Major > Labels: SalesInvoiceItemStarSchema, birt, dwh, star, starschema > > The star schema should be improved to include elements from: > * Customer dimension > * Country dimension > * Store dimension > * Catalog dimension > * Channel dimension -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OFBIZ-10991) Have a SalesChannel dimension
[ https://issues.apache.org/jira/browse/OFBIZ-10991?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16841938#comment-16841938 ] Jacques Le Roux commented on OFBIZ-10991: - Hi Pierre, Not sure why I can't get to https://issues.apache.org/jira/secure/Dashboard.jspa?selectPageId=12310603: bq. The dashboard you requested either does not exist or you don't have access to it. If you think this message is wrong, please contact your JIRA administrators. > Have a SalesChannel dimension > - > > Key: OFBIZ-10991 > URL: https://issues.apache.org/jira/browse/OFBIZ-10991 > Project: OFBiz > Issue Type: Improvement > Components: bi >Affects Versions: Trunk, Release Branch 17.12, Release Branch 18.12 >Reporter: Pierre Smits >Assignee: Pierre Smits >Priority: Major > Labels: SalesChannel, SalesChannelDimension, birt, dimension, dwh > > The component would benefit from a sales channel dimension for future fact > tables and star schema view entities -- This message was sent by Atlassian JIRA (v7.6.3#76005)