[ https://issues.apache.org/jira/browse/OFBIZ-5254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16843938#comment-16843938 ]
Jacques Le Roux edited comment on OFBIZ-5254 at 5/20/19 1:00 PM: ----------------------------------------------------------------- The issue is due to {{messageId=<565117307.0.1558356270863@LDLC>}} in commEventMap passed to updateCommunicationEvent service: {userLogin=[GenericEntity:UserLogin][createdStamp,2019-05-15 09:07:58.304(java.sql.Timestamp)][createdTxStamp,2019-05-15 09:07:57.934(java.sql.Timestamp)][currentPassword,null()][disabledBy,null()][disabledDateTime,null()][enabled,N(java.lang.String)][externalAuthId,null()][hasLoggedOut,null()][isSystem,Y(java.lang.String)][lastCurrencyUom,null()][lastLocale,null()][lastTimeZone,null()][lastUpdatedStamp,2019-05-15 09:08:02.259(java.sql.Timestamp)][lastUpdatedTxStamp,2019-05-15 09:08:02.077(java.sql.Timestamp)][partyId,system(java.lang.String)][passwordHint,null()][requirePasswordChange,null()][successiveFailedLogins,null()][userLdapDn,null()][userLoginId,system(java.lang.String)], statusId=COM_COMPLETE, entryDate=2019-05-20 14:44:30.0, subject=PD#DEMO-PRODUCT-1 - Demo Product 1, datetimeEnded=2019-05-20 14:44:30.873, communicationEventId=DEMO-COM-PRODUCT-1, messageId=<565117307.0.1558356270863@LDLC>, toString=ofbizscrumproductow...@example.com, fromString=ofbizt...@example.com, content= } LDLC is the name of my machine. The problem is the "<" ">" symboles around the messageId. I'll check why they are needed if they are... was (Author: jacques.le.roux): The issue is due to {{messageId=<565117307.0.1558356270863@LDLC>}} in commEventMap passed to updateCommunicationEvent service: {noformat} {userLogin=[GenericEntity:UserLogin][createdStamp,2019-05-15 09:07:58.304(java.sql.Timestamp)][createdTxStamp,2019-05-15 09:07:57.934(java.sql.Timestamp)][currentPassword,null()][disabledBy,null()][disabledDateTime,null()][enabled,N(java.lang.String)][externalAuthId,null()][hasLoggedOut,null()][isSystem,Y(java.lang.String)][lastCurrencyUom,null()][lastLocale,null()][lastTimeZone,null()][lastUpdatedStamp,2019-05-15 09:08:02.259(java.sql.Timestamp)][lastUpdatedTxStamp,2019-05-15 09:08:02.077(java.sql.Timestamp)][partyId,system(java.lang.String)][passwordHint,null()][requirePasswordChange,null()][successiveFailedLogins,null()][userLdapDn,null()][userLoginId,system(java.lang.String)], statusId=COM_COMPLETE, entryDate=2019-05-20 14:44:30.0, subject=PD#DEMO-PRODUCT-1 - Demo Product 1, datetimeEnded=2019-05-20 14:44:30.873, communicationEventId=DEMO-COM-PRODUCT-1, messageId=<565117307.0.1558356270863@LDLC>, toString=ofbizscrumproductow...@example.com, fromString=ofbizt...@example.com, content= } {noformat} LDLC is the name of my machine. The problem is the "<" ">" symboles around the messageId. I'll check why they are needed if they are... > 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, > 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<String> 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)