[jira] [Commented] (OFBIZ-9800) French translation of OFBiz website

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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

2019-05-17 Thread Nicolas Malin (JIRA)


 [ 
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

2019-05-17 Thread Nicolas Malin (JIRA)


[ 
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

2019-05-17 Thread Nicolas Malin (JIRA)
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

2019-05-17 Thread Nicolas Malin (JIRA)


 [ 
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

2019-05-17 Thread Nicolas Malin (JIRA)


 [ 
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

2019-05-17 Thread Nicolas Malin (JIRA)


 [ 
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

2019-05-17 Thread Nicolas Malin (JIRA)
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

2019-05-17 Thread Olivier Heintz (JIRA)


[ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


 [ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


 [ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


 [ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


 [ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


 [ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


 [ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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

2019-05-17 Thread Pierre Smits (JIRA)


[ 
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

2019-05-17 Thread Jacques Le Roux (JIRA)


 [ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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

2019-05-17 Thread Pierre Smits (JIRA)


[ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


 [ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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"

2019-05-17 Thread Jacques Le Roux (JIRA)


 [ 
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

2019-05-17 Thread Pierre Smits (JIRA)


[ 
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

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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

2019-05-17 Thread Jacques Le Roux (JIRA)


[ 
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)