You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/10/14 17:05:41 UTC
[GitHub] [commons-jxpath] kyakdan opened a new pull request, #26: Fix CVE-2022-41852
kyakdan opened a new pull request, #26:
URL: https://github.com/apache/commons-jxpath/pull/26
Those using JXPath to interpret untrusted XPath expressions may be vulnerable to a remote code execution attack. All JXPathContext class functions processing a XPath string are vulnerable except compile() and compilePath() function. The XPath expression can be used by an attacker to load any Java class from the classpath resulting in code execution.
9.8 Critical
1.3 and earlier
Remote Code Execution (RCE)
- Issue reported here is, all functions in the class JXPathContext (except compile and compilePath) are vulnerable to a remote code execution attack. - An arbitrary code can be injected in the xpath values passed to these functions, and it allows triggering java classes that can exploit the target machine.
- For instance, the iterate() method in the JXPathContext class, can be invoked by passing the xpath argument value as, java.lang.Thread.sleep(9999999) or java.lang.Class.forName("ExploitTest"). These examples can result in triggering the injected java code, and can exploit the target machine.
- Example: JXPathContext context = JXPathContext.newContext(new Test() ); Iterator result = context.iterate("java.lang.Thread.sleep(9999999)"); System.out.println("result.hasNext() - " + result.hasNext());
- No workaround available
The fix added here is via a new system property "jxpath.class.allow". This list is empty by default meaning that nothing is allowed by default. Users should then explicitly configure which classes are allowed.
- In order to fix this issue, a filter class "JXPathFilter.java" is added in JXPath. This filter object will be used to validate if the xpath passed to JXPath is a valid one
- This new filter class, for now implements JXPathClassFilter interface, which is used to check the java classes passed in xpath.
- In this filter, the class filter validates to see if the class being loaded in JXPath is part of the restriction list. The restriction can be configured via a system property "jxpath.class.allow"
- System property "jxpath.class.allow" can be set to specify the list of allowed classnames. This property takes a list of java classnames (use comma as separator to specify more than one class). If this property is not set, it disallows all classes.
- When a new extension function is created by JXPath, it uses the filter to check if the xpath supplied is a valid string, only then function instances are created.
- The changes are added to pass JXPathFilter instance as an additional argument to the methods that create the ExtensionFunction objects. In addition, the ClassLoaderUtil class is modified to use the JXPathFilter instance when a new Class instance is created. Please note that, the changes here are added as overloaded methods, so that we are not affecting existing code for anyone.
This is based on #25 and the logic is changed from a deny list into an allow list
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1286717058
@Warxim Many thanks for the detailed explanation and the PoC of still insecure methods. I've added checks to the `MethodFunciton` class that checks the declaring class before invoking the method. A unit test for the use case you described is added to verify the fix. Could you give it a try now and give feedback if you find further cases that are not yet secured?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1313947328
It is true that, similarly to SQL injections, Xpath injections are vulnerabilities in the applications that use the library. However, the application's responsibility ends when the library exposes functionality that enables propagating a vulnerability to other system components. In the case of SQL injections, the scope matters. If a crafted SQL payload would result in an injection that allows direct code execution outside the database context, we should treat this case as a security issue. There are multiple examples where libraries enabled code execution by mistake or as a feature. Those issues were addressed and fixed. Here are a few SQL examples:
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2009-4019 (MySQL)
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-3095 (SQL engine IBM DB2
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-4061 (Microsoft SQL Server)
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2015-1889 (Big SQL in IBM InfoSpere)
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2015-3414 (SQLite)
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2015-3415 (SQLite)
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2015-3416 (SQLite)
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-2520 (Apple sqllite)
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-2519 (Apple sqllite)
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-2518 (Apple sqllite)
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-2513 (Apple sqllite)
Recently, Jazzer found an issue of a similar nature in OSS-Fuzz in a HyperSQL (https://nvd.nist.gov/vuln/detail/CVE-2022-41853). The maintainer quickly acknowledged the security risk and agreed to issue a CVE and a fix release. Based on the previous arguments, we still think this is a security issue for which a CVE should be issued.
That said, I'll adjust the PR title to get it merged. Our goal here is to ensure that users get a secure version of the library and the PR title seems to be a blocker for this to happen.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] stephanborn commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by GitBox <gi...@apache.org>.
stephanborn commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1319703699
Now as @kyakdan has renamed the PR's title to "get it merged" - is there a plan / schedule when this will be done and a new version with this included will be released? It would be good if that could be communicated.
I am pretty sure there a several projects which are waiting for a release. I hope it will be done very soon as otherwise we need to replace JXPath in our project as we are not allowed to use libs with known security issues with that high criticality.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1000142254
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -60,9 +59,9 @@ public void setUp() {
lib.addFunctions(new ClassFunctions(TestFunctions2.class, "test"));
lib.addFunctions(new PackageFunctions("", "call"));
lib.addFunctions(
- new PackageFunctions(
- "org.apache.commons.jxpath.ri.compiler.",
- "jxpathtest"));
+ new PackageFunctions(
Review Comment:
This reason is my IDE auto-reformating the file. I've only kept the relevant changes that are part of this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999927379
##########
src/main/java/org/apache/commons/jxpath/ClassFunctions.java:
##########
@@ -91,6 +92,13 @@ public Function getFunction(
final String namespace,
final String name,
Object[] parameters) {
+ JXPathFilter jxPathFilter = new JXPathFilter();
Review Comment:
I think we should have some kind of interface `JXPathFilter` that will have all required methods (e.g. `isClassNameExposed(className)`) that will be used as the parameter of the overloaded method `Functions.getFunction(...)`. The overloaded method would create the `SystemPropertyJXPathFilter` implementation that uses system properties to specify filters.
Then we would add field to JXPathContext that would hold the default JXPathFilter for given context (by default our properties based filter). We would also add methods to change this filter, so that the developer can configure per-context filters.
The `JXPathFilter` itself should be either interface with all the methods or just some filter-holder that would contain all the filters (interfaces JXPathClassFilter, JXPathXyzFilter, ...), so that we can pass it through the structure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999927379
##########
src/main/java/org/apache/commons/jxpath/ClassFunctions.java:
##########
@@ -91,6 +92,13 @@ public Function getFunction(
final String namespace,
final String name,
Object[] parameters) {
+ JXPathFilter jxPathFilter = new JXPathFilter();
Review Comment:
I think we should have some kind of interface `JXPathFilter` that will have all required methods (e.g. `isClassNameExposed(className)`) that will be used as the parameter of the overloaded method `Functions.getFunction(...)`. The overloaded method would create the `SystemPropertyJXPathFilter` implementation that uses system properties to specify filters.
Then we would add field to JXPathContext that would hold the default JXPathFilter for given context (by default our system properties based filter). We would also add methods to change this filter, so that the developer can configure per-context filters.
The `JXPathFilter` itself should be either interface with all the methods or just some filter-holder that would contain all the filters (interfaces JXPathClassFilter, JXPathXyzFilter, ...), so that we can pass it through the structure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] JanHron commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
JanHron commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1305809446
> In short: no. I am a volunteer here, so I don't get paid, I have other priorities and interests; I work here on a best-effort basis based on my POV. This is the classic story of free and open-source development. Gary
@garydgregory No need to get all defensive, I did not want to start one of those discussions where a random nobody demands attention from a volunteer member of an open source project. I'm well aware of the situation and your work is very much appreciated. Apologies for the misunderstanding. My background is in security, so my expectations about what should happen whenever a critical vulnerability is found may be skewed a bit in favor of immediate action.
So, do I understand correctly that this vulnerability is as serious as it sounds - that unsanitized user input into a JXPath expression leads to getting a remote shell, or is there some caveat I'm missing? If so, would you please consider making a HotFix release of 1.3 with just this fix so that the users of `commons-jxpath` can patch their projects (many of them open-source too, BTW) and sleep well at night?
And it's not like I'm just asking you to do more unpaid work - I'm willing to help too! If there's anything I can do to help move this along, just let me know.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1004641669
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -46,9 +46,11 @@ public class ExtensionFunctionTest extends JXPathTestCase {
private JXPathContext context;
private TestBean testBean;
private TypeConverter typeConverter;
+ private final String DEFAULT_ALLOW_LIST = "org.w3c.*,org.jdom.*,java.lang.String,java.util.*,org.apache.commons.*";
Review Comment:
I am also not an expert on JXPath, but I agree that it would be great to have some secure default allow list. đź‘Ť Maybe the maintainers would be able to help with this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] ecki commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
ecki commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1302255250
We should clarify if the Whitelist on classes includes inherited methods.
Be careful with default Whitelist, String.wait() and .class/.getClass() (or generally all methods inherited from object?) should probably not be included. Similar Integer/Boolean.getProperty
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] stephanborn commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
stephanborn commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1311390087
Not sure why there are some comments on JEXL which is a different library (commons-jexl) than commons-jxpath. The PR is for JXPath. But maybe I do not get the full picture.
Yes, untrusted input could also be handled by the applications which are using commons-jxpath. But that would put the burden on hundreds of applications comming up with solutions on their own. And still they would not be sure if they did it correctly.
The PR presented here makes jxpath secure by default - and hence should be shipped as soon as possible with high priority from my point of view.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] hummelm10 commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
hummelm10 commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1311749502
> The CVE record has been updated to invalid so my request to edit the title of this PR to remove the CVE reference stands.
For clarity, this is not true. The record has been marked DISPUTED which is entirely different to REJECT per the CVE website. (https://www.cve.org/ResourcesSupport/FAQs) Disputed is when there is disagreement between a vendor and security researcher so CVE Program leaves the record as is and updates the status. Rejected is when the record was placed in error and is not a vulnerability and the record could potentially be returned to the reserved pool in the future. The CVE is currently still valid and tied to the vulnerability just with the updated status so people can further research the vulnerability themselves.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] garydgregory commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999895278
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -60,9 +59,9 @@ public void setUp() {
lib.addFunctions(new ClassFunctions(TestFunctions2.class, "test"));
lib.addFunctions(new PackageFunctions("", "call"));
lib.addFunctions(
- new PackageFunctions(
- "org.apache.commons.jxpath.ri.compiler.",
- "jxpathtest"));
+ new PackageFunctions(
Review Comment:
What's changed here? There are too many formatting changes to make this simple to review.
In general, don't reformat existing code in PRs, it just makes noise in the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] garydgregory commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999300705
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
Review Comment:
We don't use author tags. Credit it given in the changes.xml file after a PR is merged (to avoid conflicts with other PRs).
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
+ * @version $Revision$ $Date$
+ */
+public class JXPathFilter implements JXPathClassFilter {
+ private Set<String> allowedClassesList = null;
+
+ public JXPathFilter() {
+ init();
+ }
+
+ public void init() {
+ String restrictedClasses = System.getProperty("jxpath.class.allow");
Review Comment:
Use `final` where you can.
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
+ * @version $Revision$ $Date$
+ */
Review Comment:
New public and protected elements should have a Javadoc `since` tag for the next version, which should be `1.4`.
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/TestFunctions3.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri.compiler;
+
+import org.apache.commons.jxpath.Functions;
+import org.apache.commons.jxpath.JXPathContext;
+
+import java.util.*;
Review Comment:
Don't use star imports.
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
+ * @version $Revision$ $Date$
+ */
+public class JXPathFilter implements JXPathClassFilter {
+ private Set<String> allowedClassesList = null;
+
+ public JXPathFilter() {
+ init();
+ }
+
+ public void init() {
+ String restrictedClasses = System.getProperty("jxpath.class.allow");
+ allowedClassesList = null;
+ if (restrictedClasses != null && restrictedClasses.trim().length() > 0) {
Review Comment:
Use `String#isEmpty()`
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
+ * @version $Revision$ $Date$
Review Comment:
We don't use Subversion anymore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1002983191
##########
src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java:
##########
@@ -34,6 +36,8 @@ public class MethodFunction implements Function {
private final Method method;
private static final Object[] EMPTY_ARRAY = {};
+ private JXPathFilter jxpathFilter = new SystemPropertyJXPathFilter();
Review Comment:
This creation should not happen here. Maybe add second constructor with filter parameter and use value from [...]Functions. In the constructor without this parameter, call `this(method, new SystemPropertyJXPathFilter())`.
In [...]Functions classes, create the methodFunction using the new constructor.
##########
src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ */
+public class SystemPropertyJXPathFilter implements JXPathFilter {
+ private final Set<String> allowedClasses;
+
+ public SystemPropertyJXPathFilter() {
+ this.allowedClasses = new HashSet<>();
+ final String allowedClasses = System.getProperty("jxpath.class.allow");
+ if (allowedClasses != null && !allowedClasses.isEmpty()) {
+ this.allowedClasses.addAll(Arrays.asList(allowedClasses.split(",")));
+ }
+ }
+
+ /**
+ * Specifies whether the Java class of the specified name be exposed via xpath
+ *
+ * @param className is the fully qualified name of the java class being checked.
+ * This will not be null. Only non-array class names will be passed.
+ * @return true if the java class can be exposed via xpath, false otherwise
+ */
+ @Override
+ public boolean isClassNameExposed(String className) {
+ if (allowedClasses.isEmpty()) {
+ System.err.println(false);
Review Comment:
Debugging code?
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+/**
+ * Class filter (optional) to be used by JXPath.
+ *
+ * System property "jxpath.class.allow" can be set to specify the list of allowed classnames.
+ * This property takes a list of java classnames (use comma as separator to specify more than one class).
+ * If this property is not set, it exposes no java classes
+ * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other
+ * classes will be not exposed. You can use the wildcard (*) to allow all classes.
+ * @since 1.4
+ */
+public interface JXPathFilter {
Review Comment:
We might also consider creating two implementations:
- one for allowing all paths by default (with huge security warning in javadoc) - it will always return `true`
- one for programmatically defining allowed classNames, e.g. `CustomJXPathFilter` (with the `Set<String> allowedClasses` as constructor parameter) - most of the code from `SystemPropertyJXPathFilter` can be then moved to abstract class and be used in both CustomJXPathFilter and SystemPropertyJXPathFilter
##########
src/main/java/org/apache/commons/jxpath/Functions.java:
##########
@@ -43,4 +45,17 @@ public interface Functions {
* @return Function
*/
Function getFunction(String namespace, String name, Object[] parameters);
+
+ /**
+ * Returns a Function, if any, for the specified namespace,
+ * name and parameter types if the function is allowed by the filter.
+ * @param namespace ns
+ * @param name function name
+ * @param parameters Object[]
+ * @param filter JXPathFilter
+ * @return Function
+ */
+ default Function getFunction(String namespace, String name, Object[] parameters, JXPathFilter filter) {
Review Comment:
This will allow the developer to specify custom filter on context level.
##########
src/main/java/org/apache/commons/jxpath/Functions.java:
##########
@@ -43,4 +45,17 @@ public interface Functions {
* @return Function
*/
Function getFunction(String namespace, String name, Object[] parameters);
+
+ /**
+ * Returns a Function, if any, for the specified namespace,
+ * name and parameter types if the function is allowed by the filter.
+ * @param namespace ns
+ * @param name function name
+ * @param parameters Object[]
+ * @param filter JXPathFilter
+ * @return Function
+ */
+ default Function getFunction(String namespace, String name, Object[] parameters, JXPathFilter filter) {
Review Comment:
Please add `JXPathFilter` as field to `JXPathContext` with the default `new SystemPropertyJXPathFilter()` value. Also add `getPathFilter` and `setPathFilter` methods to it.
After that, call this `Functions.getFunction` method from `JXPathContextReferenceImpl` with the field value as filter parameter.
Also implement this method in `FunctionLibrary` and `PackageFunctions`, so that the filter parameter can be used for `MethodFunction::new` and `ClassLoaderUtil.getClass`.
##########
src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ */
+public class SystemPropertyJXPathFilter implements JXPathFilter {
+ private final Set<String> allowedClasses;
+
+ public SystemPropertyJXPathFilter() {
+ this.allowedClasses = new HashSet<>();
Review Comment:
Since it is not allowed to modify this, we might consider using unmodifiable set
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -46,9 +46,11 @@ public class ExtensionFunctionTest extends JXPathTestCase {
private JXPathContext context;
private TestBean testBean;
private TypeConverter typeConverter;
+ private final String DEFAULT_ALLOW_LIST = "org.w3c.*,org.jdom.*,java.lang.String,java.util.*,org.apache.commons.*";
Review Comment:
**Note:** New Commons JXPath version will not be automatically compatible with the previous one, if the developers use functions in xpaths. All function calls will be disabled by default in the new version.
This needs to be emphasized in changelog or somewhere.
##########
src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ */
+public class SystemPropertyJXPathFilter implements JXPathFilter {
+ private final Set<String> allowedClasses;
+
+ public SystemPropertyJXPathFilter() {
+ this.allowedClasses = new HashSet<>();
+ final String allowedClasses = System.getProperty("jxpath.class.allow");
+ if (allowedClasses != null && !allowedClasses.isEmpty()) {
+ this.allowedClasses.addAll(Arrays.asList(allowedClasses.split(",")));
+ }
+ }
+
+ /**
+ * Specifies whether the Java class of the specified name be exposed via xpath
+ *
+ * @param className is the fully qualified name of the java class being checked.
+ * This will not be null. Only non-array class names will be passed.
+ * @return true if the java class can be exposed via xpath, false otherwise
+ */
+ @Override
+ public boolean isClassNameExposed(String className) {
+ if (allowedClasses.isEmpty()) {
+ System.err.println(false);
+ return false;
+ }
+
+ if (allowedClasses.contains("*")) {
+ System.err.println(true);
Review Comment:
Debugging code?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1003034601
##########
src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java:
##########
@@ -34,6 +36,8 @@ public class MethodFunction implements Function {
private final Method method;
private static final Object[] EMPTY_ARRAY = {};
+ private JXPathFilter jxpathFilter = new SystemPropertyJXPathFilter();
Review Comment:
It would be probably better to add the same filtering mechanism to `ConstructorFunction`. I think it is not vulnerable now, but we might do this, just to be sure it will not become vulnerable in new versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] madrob commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
madrob commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r998470797
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java:
##########
@@ -27,23 +27,10 @@
import java.util.Vector;
import java.util.Map.Entry;
-import org.apache.commons.jxpath.CompiledExpression;
-import org.apache.commons.jxpath.ExceptionHandler;
-import org.apache.commons.jxpath.Function;
-import org.apache.commons.jxpath.Functions;
-import org.apache.commons.jxpath.JXPathContext;
-import org.apache.commons.jxpath.JXPathException;
-import org.apache.commons.jxpath.JXPathFunctionNotFoundException;
-import org.apache.commons.jxpath.JXPathInvalidSyntaxException;
-import org.apache.commons.jxpath.JXPathNotFoundException;
-import org.apache.commons.jxpath.JXPathTypeConversionException;
-import org.apache.commons.jxpath.Pointer;
+import org.apache.commons.jxpath.*;
Review Comment:
The project does not appear to use wildcard imports, let's stick with existing conventions for now.
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
+ * @version $Revision$ $Date$
+ */
+public class JXPathFilter implements JXPathClassFilter {
+ ArrayList<String> allowedClassesList = null;
Review Comment:
This could probably be a Set.
##########
src/test/java/org/apache/commons/jxpath/ri/model/BeanModelTestCase.java:
##########
@@ -44,14 +44,23 @@
public abstract class BeanModelTestCase extends JXPathTestCase {
private JXPathContext context;
- public void setUp() {
+ @Override
+ public void setUp() throws Exception {
// if (context == null) {
- context = JXPathContext.newContext(createContextBean());
- context.setLocale(Locale.US);
- context.setFactory(getAbstractFactory());
+ super.setUp();
+ System.setProperty("jxpath.class.allow", "*");
+ context = JXPathContext.newContext(createContextBean());
+ context.setLocale(Locale.US);
+ context.setFactory(getAbstractFactory());
// }
}
+ @Override
+ public void tearDown() throws Exception {
+ super.tearDown();
Review Comment:
Very minor, should call super.tearDown at the end of the method not the beginning.
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+/**
+ * Class filter (optional) to be used by JXPath.
+ *
+ * System property "jxpath.class.allow" can be set to specify the list of allowd classnames.
+ * This property takes a list of java classnames (use comma as separator to specify more than one class).
+ * If this property is not set, it exposes no java classes
+ * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other
Review Comment:
Should also document the wildcard * for allowing all classes.
##########
src/main/java/org/apache/commons/jxpath/ClassFunctions.java:
##########
@@ -92,6 +93,28 @@ public Function getFunction(
String namespace,
String name,
Object[] parameters) {
+ return getFunction(namespace, name, parameters, null);
+ }
+
+ public Function getFunction(
+ String namespace,
+ String name,
+ Object[] parameters,
+ JXPathFilter jxPathFilter) {
+
+ // give chance to ClassFilter to filter out, if present
+ try {
+ if (jxPathFilter != null && !jxPathFilter.exposeToXPath(functionClass.getName())) {
+ throw new ClassNotFoundException(functionClass.getName());
Review Comment:
We shouldn't be using exceptions for control flow here, let's throw JXPathException directly.
##########
src/main/java/org/apache/commons/jxpath/FunctionLibrary.java:
##########
@@ -77,12 +79,30 @@ public Set getUsedNamespaces() {
*/
public Function getFunction(String namespace, String name,
Object[] parameters) {
+ return getFunction(namespace, name, parameters, null);
Review Comment:
I worry that people will continue calling this method on upgrade and continue to be insecure by default. Instead of passing null, I think a better default would be to pass either the environment-based of a fail-closed (deny all) implementation of JXPathFilter. Same logic applies for other places where we overload the method signatures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999896659
##########
src/main/java/org/apache/commons/jxpath/ClassFunctions.java:
##########
@@ -91,6 +92,13 @@ public Function getFunction(
final String namespace,
final String name,
Object[] parameters) {
+ JXPathFilter jxPathFilter = new JXPathFilter();
Review Comment:
Not sure, but I am a little bit worried that this can be quite limiting to developers. We force them to use properties in order to allow some classes in jxpath functions. Also, it is not possible to change the filter per JXContext
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] garydgregory commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1283849870
@kyakdan I'm not sure what happened on your rebase but you should not have to edit pom.xml or changes.xml.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1288545875
@garydgregory @Warxim @madrob I've address your review. Could you have look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] JanHron commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
JanHron commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1311547394
@markt-asf I think a reasonable approach would be to make the default methods safe and provide an unsecured alternative for people who really need this functionality, with a big fat warning about injection risk. Thus, the casual users would be protected and the power users would still have all the tools they need. I like your idea about prepared statements too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] supermaurio commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by "supermaurio (via GitHub)" <gi...@apache.org>.
supermaurio commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1463808132
@kyakdan in the current form, every class requested from ClassLoaderUtil needs to be whitelisted.
This means that by default, even basic methods like JXPathContext.newContext() will fail (as org.apache.commons.jxpath.ri.JXPathContextFactoryReferenceImpl will be filtered by default)
Is this intended behaviour?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] garydgregory commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999316059
##########
src/main/java/org/apache/commons/jxpath/Functions.java:
##########
@@ -43,4 +45,15 @@ public interface Functions {
* @return Function
*/
Function getFunction(String namespace, String name, Object[] parameters);
+
+ /**
+ * Returns a Function, if any, for the specified namespace,
+ * name and parameter types.
+ * @param namespace ns
+ * @param name function name
+ * @param parameters Object[]
+ * @param jxPathFilter the XPath filter
+ * @return Function
+ */
+ Function getFunction(String namespace, String name, Object[] parameters, JXPathFilter jxPathFilter);
Review Comment:
This needs to be a default method to avoid breaking binary compatibility. This is a misconfiguration in our parent POM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999887762
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ */
+public class JXPathFilter implements JXPathClassFilter {
+ private Set<String> allowedClassesList = null;
Review Comment:
Good catch. Changed. I've also simplified the constructor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] ecki commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by "ecki (via GitHub)" <gi...@apache.org>.
ecki commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1474461889
I don’t think „most“ is correct, since this was not much requested before and many users probably only issue static jxpath expressions or use it as a cheap java eval language anyway.
having said that, your mentioning of the FunctionLibrary is on point, and it also looks like a better way to filter. Jxpath can just ship (configurable) alternatives, which especially have allow lists for functions, can turn java evaluation off or restrict classes and methods (similar to like the PR does, ideally without the need to allow internal methods)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1007189707
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -46,9 +46,11 @@ public class ExtensionFunctionTest extends JXPathTestCase {
private JXPathContext context;
private TestBean testBean;
private TypeConverter typeConverter;
+ private final String DEFAULT_ALLOW_LIST = "org.w3c.*,org.jdom.*,java.lang.String,java.util.*,org.apache.commons.*";
Review Comment:
@garydgregory As the current state of the PR fully implements the allow-list approach that denies loading any classes by default when nothing is configured, I suggest merging it so that a new release of JXPath that fixes the CVE can be published. This way users need to update their configurations with the classes they want to allow. In a follow-up change, we can introduce a default allow list of sane defaults based on the feedback we get from the users.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1308189293
We do not agree here. Xpath injections are pretty common, see https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=xpath. The past shows that a lot of applications from well-known companies were vulnerable to XPath injections when an attacker was able to control the XPath expression completely or partially. Sanitization of user input failed in multiple cases. It might be expected by developers that the worst-case scenario resulting from an XPath injection would be disclosing sensitive information, but no one would usually expect arbitrary code execution. In Apache Commons JXPath it is enough to have partial control of the XPath expression to call methods from an arbitrary class. @Warxim has demonstrated how easily an exploit might work, see https://hackinglab.cz/en/blog/remote-code-execution-in-jxpath-library-cve-2022-41852/. As a maintainer of a popular open-source library, you have no control over how users might use the library.
We would appreciate it if apache-commons would take more responsibility here and start notifying users about the potential security risks related to untrusted input vectors instead of merely saying "library is not intended to parse untrusted input". The CVE description is valid since it clearly says "**Those using JXPath to interpret untrusted XPath expressions** may be vulnerable to a remote code execution attack". We are not arguing about an internal unexposed functionality here, but about an actual feature of the library that can have serious security implications.
If you don't plan to assign CVEs for issues resulting from untrusted user input or plan to reject valid ones, you should consider adding a clear warning on the official project page along the lines: "Attention: The library is not intended to be used with untrusted input. Untrusted input could lead to Remote Code Execution, Denial of Service or other unexpected behavior". That would help developers to be aware of the potential security risks or consider switching to other libraries that are robust against untrusted input data. We also suggest adding such a warning to all apache-commons libraries that are _not intended_ to be used with untrusted user input. Some small notes in the documentation are not enough to raise the required attention. In the case of JXpath, neither the current project page (https://github.com/apache/commons-jxpath) nor the user guide (https://commons.apache.org/proper/commons-jxpath/users-guide.html) contains any information that JXPath cannot safely handle u
ntrusted input.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] markt-asf commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
markt-asf commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1311500839
Sorry about the JEXL / JXPath mix-up. Trying to respond to lots of similar issues about projects with similar names.
The CVE record has been updated to invalid so my request to edit the title of this PR to remove the CVE reference stands.
My previous comments stand for JXPath. It is an expression language intended to manipulate (quoting from the project description) "graphs of objects of all kinds: JavaBeans, Maps, Servlet contexts, DOM etc, including mixtures thereof." It is not reasonable to expect a tool designed for that purpose to safely handle untrusted input unless using an API explicitly documented to do so. Hence this is not a security issue and will not be treated as such.
Treating an application's XPath injection vulnerability as a issue in JXPath is equivalent to calling an application's SQL injection vulnerability an issue in the JDBC driver. The vulnerability is in the application, not the library the application is (ab)using.
I have concerns about a filter approach for JXPath since experience across multiple projects has shown that trying to limit such tools to a subset of safe operations is difficult and typically leads to a long series issues being reported as researchers find loopholes that allow the filtering to be bypassed. However, those approaches often used deny lists whereas this PR uses an allow list so it is starting from a much safer position.
I wonder if a JXPath equivalent (and it could be a very loose equivalent) to prepared statements in the SQL world could be useful.
Generally, if the community consensus is to add some form of feature to handle unsafe input then it needs to be clearly documented for that purpose and the community has to be prepared to handle security vulnerability reports against that feature.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1284500165
@madrob @garydgregory Thanks for the review.
@garydgregory I've rebased again and the PR does not contain changes to pom.xml or changes.xml now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r998936907
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
+ * @version $Revision$ $Date$
+ */
+public class JXPathFilter implements JXPathClassFilter {
+ ArrayList<String> allowedClassesList = null;
Review Comment:
Changed!
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathContextReferenceImpl.java:
##########
@@ -27,23 +27,10 @@
import java.util.Vector;
import java.util.Map.Entry;
-import org.apache.commons.jxpath.CompiledExpression;
-import org.apache.commons.jxpath.ExceptionHandler;
-import org.apache.commons.jxpath.Function;
-import org.apache.commons.jxpath.Functions;
-import org.apache.commons.jxpath.JXPathContext;
-import org.apache.commons.jxpath.JXPathException;
-import org.apache.commons.jxpath.JXPathFunctionNotFoundException;
-import org.apache.commons.jxpath.JXPathInvalidSyntaxException;
-import org.apache.commons.jxpath.JXPathNotFoundException;
-import org.apache.commons.jxpath.JXPathTypeConversionException;
-import org.apache.commons.jxpath.Pointer;
+import org.apache.commons.jxpath.*;
Review Comment:
Changed.
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathClassFilter.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+/**
+ * Class filter (optional) to be used by JXPath.
+ *
+ * System property "jxpath.class.allow" can be set to specify the list of allowd classnames.
+ * This property takes a list of java classnames (use comma as separator to specify more than one class).
+ * If this property is not set, it exposes no java classes
+ * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other
Review Comment:
Changed!
##########
src/test/java/org/apache/commons/jxpath/ri/model/BeanModelTestCase.java:
##########
@@ -44,14 +44,23 @@
public abstract class BeanModelTestCase extends JXPathTestCase {
private JXPathContext context;
- public void setUp() {
+ @Override
+ public void setUp() throws Exception {
// if (context == null) {
- context = JXPathContext.newContext(createContextBean());
- context.setLocale(Locale.US);
- context.setFactory(getAbstractFactory());
+ super.setUp();
+ System.setProperty("jxpath.class.allow", "*");
+ context = JXPathContext.newContext(createContextBean());
+ context.setLocale(Locale.US);
+ context.setFactory(getAbstractFactory());
// }
}
+ @Override
+ public void tearDown() throws Exception {
+ super.tearDown();
Review Comment:
changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] garydgregory commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1302144809
On Thu, Nov 3, 2022 at 8:43 AM JanHron ***@***.***> wrote:
> @garydgregory <https://github.com/garydgregory> Apologies for being so
> blunt, but don't you think a few weeks might be a bit too long for such a
> high-severity issue? Maybe I misunderstood the ease of exploiting it, but
> it seems like quite a nasty vulnerability.
>
In short: no. I am a volunteer here, so I don't get paid, I have other
priorities and interests; I work here on a best-effort basis based on my
POV. This is the classic story of free and open-source development.
Gary
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-jxpath/pull/26#issuecomment-1302042765>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJB6N7HH3D4T2GGHHSFPADWGOXP3ANCNFSM6AAAAAARFNKCMY>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1304399134
> We should clarify if the Whitelist on classes includes inherited methods.
>
> Be careful with default Whitelist, String.wait() and .class/.getClass() (or generally all methods inherited from object?) should probably not be included. Similar Integer/Boolean.getProperty
Good point! The current implementation does not allow any classes if not otherwise configured by the user. We can merge and release this variant so that users can use get a secure version of the library. A default secure allow-list can be done in a follow-up update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] stephanborn commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
stephanborn commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1302069798
If releasing a 1.3.1 with this fix only would speed up review and release procedures then I support this. A version 1.4 with this fix and all other already applied changes could be done later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] markt-asf commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
markt-asf commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1307567283
CVE-2022-41852 was issued in error and is in the process of being updated so the state becomes "rejected" / "disputed" / "issued in error" or whatever is the most appropriate state.
There is no security vulnerability to be fixed here.
JEXL is NOT expected to safely handle untrusted input.
Whether a better set of defaults can be provided to help users avoid shooting themselves in the foot is a different question. And a reasonable aim for this or any other PR.
Please update the title of this PR to something more appropriate that does not suggest that a security vulnerability exists here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] markt-asf commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by GitBox <gi...@apache.org>.
markt-asf commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1319707214
There is no security vulnerability. This PR will be dealt with with the same priority as any other enhancement request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] garydgregory commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1294098925
I might take time to review this weekend but it could be a few weeks before a release candidate is ready.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999615893
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
Review Comment:
Removed!
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
+ * @version $Revision$ $Date$
Review Comment:
Changed to `@since 1.4`
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
+ * @version $Revision$ $Date$
+ */
Review Comment:
Done!
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
+ * @version $Revision$ $Date$
+ */
+public class JXPathFilter implements JXPathClassFilter {
+ private Set<String> allowedClassesList = null;
+
+ public JXPathFilter() {
+ init();
+ }
+
+ public void init() {
+ String restrictedClasses = System.getProperty("jxpath.class.allow");
+ allowedClassesList = null;
+ if (restrictedClasses != null && restrictedClasses.trim().length() > 0) {
Review Comment:
Changed
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ *
+ * @author bhmohanr-techie
+ * @version $Revision$ $Date$
+ */
+public class JXPathFilter implements JXPathClassFilter {
+ private Set<String> allowedClassesList = null;
+
+ public JXPathFilter() {
+ init();
+ }
+
+ public void init() {
+ String restrictedClasses = System.getProperty("jxpath.class.allow");
Review Comment:
ACK
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/TestFunctions3.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri.compiler;
+
+import org.apache.commons.jxpath.Functions;
+import org.apache.commons.jxpath.JXPathContext;
+
+import java.util.*;
Review Comment:
Removed. The import was not used anyway.
##########
src/main/java/org/apache/commons/jxpath/Functions.java:
##########
@@ -43,4 +45,15 @@ public interface Functions {
* @return Function
*/
Function getFunction(String namespace, String name, Object[] parameters);
+
+ /**
+ * Returns a Function, if any, for the specified namespace,
+ * name and parameter types.
+ * @param namespace ns
+ * @param name function name
+ * @param parameters Object[]
+ * @param jxPathFilter the XPath filter
+ * @return Function
+ */
+ Function getFunction(String namespace, String name, Object[] parameters, JXPathFilter jxPathFilter);
Review Comment:
This method has different implementations for the three classes implementing the `Functions` interface, it cannot be made final. So, in order to not break binary compatibility, I've removed this function and made the implemented `getFunction` methods use a filter based on the `jxpath.allow.class` system property. This way we don't have to explicitly pass a filter parameter to the `getFunction` method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999879987
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ */
+public class JXPathFilter implements JXPathClassFilter {
+ private Set<String> allowedClassesList = null;
Review Comment:
Maybe rename the set to just `allowedClasses`, since it is not list anymore?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1003511821
##########
src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ */
+public class SystemPropertyJXPathFilter implements JXPathFilter {
+ private final Set<String> allowedClasses;
+
+ public SystemPropertyJXPathFilter() {
+ this.allowedClasses = new HashSet<>();
+ final String allowedClasses = System.getProperty("jxpath.class.allow");
+ if (allowedClasses != null && !allowedClasses.isEmpty()) {
+ this.allowedClasses.addAll(Arrays.asList(allowedClasses.split(",")));
+ }
+ }
+
+ /**
+ * Specifies whether the Java class of the specified name be exposed via xpath
+ *
+ * @param className is the fully qualified name of the java class being checked.
+ * This will not be null. Only non-array class names will be passed.
+ * @return true if the java class can be exposed via xpath, false otherwise
+ */
+ @Override
+ public boolean isClassNameExposed(String className) {
+ if (allowedClasses.isEmpty()) {
+ System.err.println(false);
Review Comment:
Yes. Removed!
##########
src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ */
+public class SystemPropertyJXPathFilter implements JXPathFilter {
+ private final Set<String> allowedClasses;
+
+ public SystemPropertyJXPathFilter() {
+ this.allowedClasses = new HashSet<>();
+ final String allowedClasses = System.getProperty("jxpath.class.allow");
+ if (allowedClasses != null && !allowedClasses.isEmpty()) {
+ this.allowedClasses.addAll(Arrays.asList(allowedClasses.split(",")));
+ }
+ }
+
+ /**
+ * Specifies whether the Java class of the specified name be exposed via xpath
+ *
+ * @param className is the fully qualified name of the java class being checked.
+ * This will not be null. Only non-array class names will be passed.
+ * @return true if the java class can be exposed via xpath, false otherwise
+ */
+ @Override
+ public boolean isClassNameExposed(String className) {
+ if (allowedClasses.isEmpty()) {
+ System.err.println(false);
+ return false;
+ }
+
+ if (allowedClasses.contains("*")) {
+ System.err.println(true);
Review Comment:
Yes. Removed!
##########
src/main/java/org/apache/commons/jxpath/ri/SystemPropertyJXPathFilter.java:
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
+
+/**
+ * A filter to be used by JXPath, to evaluate the xpath string values to impose any restrictions.
+ * This class implements specific filter interfaces, and implements methods in those.
+ * For instance, it JXPathClassFilter interface, which is used to check if any restricted java classes are passed via xpath
+ * JXPath uses this filter instance when an extension function instance is created.
+ */
+public class SystemPropertyJXPathFilter implements JXPathFilter {
+ private final Set<String> allowedClasses;
+
+ public SystemPropertyJXPathFilter() {
+ this.allowedClasses = new HashSet<>();
Review Comment:
Changed!
##########
src/main/java/org/apache/commons/jxpath/functions/MethodFunction.java:
##########
@@ -34,6 +36,8 @@ public class MethodFunction implements Function {
private final Method method;
private static final Object[] EMPTY_ARRAY = {};
+ private JXPathFilter jxpathFilter = new SystemPropertyJXPathFilter();
Review Comment:
Done!
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -46,9 +46,11 @@ public class ExtensionFunctionTest extends JXPathTestCase {
private JXPathContext context;
private TestBean testBean;
private TypeConverter typeConverter;
+ private final String DEFAULT_ALLOW_LIST = "org.w3c.*,org.jdom.*,java.lang.String,java.util.*,org.apache.commons.*";
Review Comment:
One idea would be to have a list of classes/packages that are deemed safe and thus should be allowed. This would mean if the system property is not configured, the default filter will be initialized with this list. This way we can guarantee a smooth update for most users while keeping them safe. As I am not an expert on JXPath, is there such a list that you can provide?
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+/**
+ * Class filter (optional) to be used by JXPath.
+ *
+ * System property "jxpath.class.allow" can be set to specify the list of allowed classnames.
+ * This property takes a list of java classnames (use comma as separator to specify more than one class).
+ * If this property is not set, it exposes no java classes
+ * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other
+ * classes will be not exposed. You can use the wildcard (*) to allow all classes.
+ * @since 1.4
+ */
+public interface JXPathFilter {
Review Comment:
- From a security perspective, I think we should be really cautious about an allow-all filter. Some users might decide to use this one to get rid of the exception they get while using the secure version, leaving them vulnerable. Such scenarios are not rare. That is why I suggested having a allow list of sane defaults.
- Added a `CustomJXPathFilter` class as you suggested.
##########
src/main/java/org/apache/commons/jxpath/Functions.java:
##########
@@ -43,4 +45,17 @@ public interface Functions {
* @return Function
*/
Function getFunction(String namespace, String name, Object[] parameters);
+
+ /**
+ * Returns a Function, if any, for the specified namespace,
+ * name and parameter types if the function is allowed by the filter.
+ * @param namespace ns
+ * @param name function name
+ * @param parameters Object[]
+ * @param filter JXPathFilter
+ * @return Function
+ */
+ default Function getFunction(String namespace, String name, Object[] parameters, JXPathFilter filter) {
Review Comment:
Excellent idea! Adjusted. I've also adjusted the tests to set a filter on the context level.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] JanHron commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
JanHron commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1302042765
@garydgregory Apologies for being so blunt, but don't you think a few weeks might be a bit too long for such a high-severity issue? Maybe I misunderstood the ease of exploiting it, but it seems like quite a nasty vulnerability.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] ecki commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
ecki commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1305849264
As I understand it, it is about untrusted expressions not about untrusted payload instances or object trees. But I could be wrong, I’d it both?
--Â https://bernd.eckenfels.net
 From: JanHron ***@***.***>Sent: Monday, November 7, 2022 4:49 PMTo: apache/commons-jxpath ***@***.***>Cc: Bernd ***@***.***>; Comment ***@***.***>Subject: Re: [apache/commons-jxpath] Fix CVE-2022-41852 (PR #26)Â
In short: no. I am a volunteer here, so I don't get paid, I have other priorities and interests; I work here on a best-effort basis based on my POV. This is the classic story of free and open-source development. Gary
@garydgregory No need to get all defensive, I did not want to start one of those discussions where a random nobody demands attention from a volunteer member of an open source project. I'm well aware of the situation and your work is very much appreciated. Apologies for the misunderstanding. My background is in security, so my expectations about what should happen whenever a critical vulnerability is found may be skewed a bit in favor of immediate action.
So, do I understand correctly that this vulnerability is as serious as it sounds - that unsanitized user input into a JXPath expression leads to getting a remote shell, or is there some caveat I'm missing? If so, would you please consider making a HotFix release of 1.3 with just this fix so that the users of commons-jxpath can patch their projects (many of them open-source too, BTW) and sleep well at night?
And it's not like I'm just asking you to do more unpaid work - I'm willing to help too! If there's anything I can do to help move this along, just let me know.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: ***@***.***>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by "kyakdan (via GitHub)" <gi...@apache.org>.
kyakdan commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1473568115
> @kyakdan in the current form, every class requested from ClassLoaderUtil needs to be whitelisted. This means that by default, even basic methods like JXPathContext.newContext() will fail (as org.apache.commons.jxpath.ri.JXPathContextFactoryReferenceImpl will be filtered by default) Is this intended behaviour?
You can provide a pattern to allow a group of classes, e.g., `org.w3c.*`. This allows all classes in the `org.w3c` package. You can use `*` as a pattern to allow everything which matches the current behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r998912350
##########
src/main/java/org/apache/commons/jxpath/ClassFunctions.java:
##########
@@ -92,6 +93,28 @@ public Function getFunction(
String namespace,
String name,
Object[] parameters) {
+ return getFunction(namespace, name, parameters, null);
+ }
+
+ public Function getFunction(
+ String namespace,
+ String name,
+ Object[] parameters,
+ JXPathFilter jxPathFilter) {
+
+ // give chance to ClassFilter to filter out, if present
+ try {
+ if (jxPathFilter != null && !jxPathFilter.exposeToXPath(functionClass.getName())) {
+ throw new ClassNotFoundException(functionClass.getName());
Review Comment:
Good point! Changed to throwing a JXPathException directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r998931265
##########
src/main/java/org/apache/commons/jxpath/FunctionLibrary.java:
##########
@@ -77,12 +79,30 @@ public Set getUsedNamespaces() {
*/
public Function getFunction(String namespace, String name,
Object[] parameters) {
+ return getFunction(namespace, name, parameters, null);
Review Comment:
Excellent point. Changed all usages to use a default value of the JXPathFilter that is initialized to the filter set by the system property of the allow list. This guarantees consistent behavior among all method calls.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1002987643
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -46,9 +46,11 @@ public class ExtensionFunctionTest extends JXPathTestCase {
private JXPathContext context;
private TestBean testBean;
private TypeConverter typeConverter;
+ private final String DEFAULT_ALLOW_LIST = "org.w3c.*,org.jdom.*,java.lang.String,java.util.*,org.apache.commons.*";
Review Comment:
**Note:** New Commons JXPath version will not be automatically compatible with the previous one, if the developers use functions in xpaths. All function calls will be disabled by default in the new version. (For example, calling `size(/)` will not be possible without first allowing it in filter.)
This needs to be emphasized in changelog or somewhere.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1002987643
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -46,9 +46,11 @@ public class ExtensionFunctionTest extends JXPathTestCase {
private JXPathContext context;
private TestBean testBean;
private TypeConverter typeConverter;
+ private final String DEFAULT_ALLOW_LIST = "org.w3c.*,org.jdom.*,java.lang.String,java.util.*,org.apache.commons.*";
Review Comment:
**Warning:** New Commons JXPath version with the changes in this PR will not be automatically compatible with the previous one, if the developers use functions in xpaths. All function calls will be disabled by default in the new version. (For example, calling `size(/)` will not be possible without first allowing it in filter.)
This needs to be emphasized in changelog or somewhere.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1002987643
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -46,9 +46,11 @@ public class ExtensionFunctionTest extends JXPathTestCase {
private JXPathContext context;
private TestBean testBean;
private TypeConverter typeConverter;
+ private final String DEFAULT_ALLOW_LIST = "org.w3c.*,org.jdom.*,java.lang.String,java.util.*,org.apache.commons.*";
Review Comment:
**Warning:** New Commons JXPath version with these changes will not be automatically compatible with the previous one, if the developers use functions in xpaths. All function calls will be disabled by default in the new version. (For example, calling `size(/)` will not be possible without first allowing it in filter.)
This needs to be emphasized in changelog or somewhere.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1003005419
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+/**
+ * Class filter (optional) to be used by JXPath.
+ *
+ * System property "jxpath.class.allow" can be set to specify the list of allowed classnames.
+ * This property takes a list of java classnames (use comma as separator to specify more than one class).
+ * If this property is not set, it exposes no java classes
+ * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other
+ * classes will be not exposed. You can use the wildcard (*) to allow all classes.
+ * @since 1.4
+ */
+public interface JXPathFilter {
Review Comment:
We might also consider creating two implementations:
- one for allowing all paths by default (with huge security warning in javadoc) - it will always return `true`
- one for programmatically defining allowed classNames, e.g. `CustomJXPathFilter` (with the `Set<String> allowedClasses` as constructor parameter). Most of the code from `SystemPropertyJXPathFilter` can be then moved to abstract class and be used in both CustomJXPathFilter and SystemPropertyJXPathFilter. In `CustomJXPathFilter`, we might just call `super(Collections.unmodifiableSet(allowedClasses));` and in `SystemPropertyJXPathFilter`, we might just call `super(loadAllowedClassesFromSystemProperty());`.
##########
src/main/java/org/apache/commons/jxpath/ri/JXPathFilter.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jxpath.ri;
+
+/**
+ * Class filter (optional) to be used by JXPath.
+ *
+ * System property "jxpath.class.allow" can be set to specify the list of allowed classnames.
+ * This property takes a list of java classnames (use comma as separator to specify more than one class).
+ * If this property is not set, it exposes no java classes
+ * Ex: jxpath.class.allow=java.lang.Runtime will allow exposing java.lang.Runtime class via xpath, while all other
+ * classes will be not exposed. You can use the wildcard (*) to allow all classes.
+ * @since 1.4
+ */
+public interface JXPathFilter {
Review Comment:
We might also consider creating two implementations:
- One for allowing all paths by default (with huge security warning in javadoc) - it will always return `true`.
- One for programmatically defining allowed classNames, e.g. `CustomJXPathFilter` (with the `Set<String> allowedClasses` as constructor parameter). Most of the code from `SystemPropertyJXPathFilter` can be then moved to abstract class and be used in both CustomJXPathFilter and SystemPropertyJXPathFilter. In `CustomJXPathFilter`, we might just call `super(Collections.unmodifiableSet(allowedClasses));` and in `SystemPropertyJXPathFilter`, we might just call `super(loadAllowedClassesFromSystemProperty());`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] ecki commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
ecki commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1311191317
> JEXL is NOT expected to safely handle untrusted input.
Be careful, this is about xpath, and also even Jexl should handle data securely, it’s just a question if expressions/programs should be trusted or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] oliverchang commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by GitBox <gi...@apache.org>.
oliverchang commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1321443964
@markt-asf I understand if this is not in the threat model for JXPath. However, I can't seem to find any documentation explicitly stating JXPath should only be used to process trusted input, which could lead to user confusion and incorrect usages (which includes our fuzzing efforts). Would it make sense to update the documentation to make this fact very clear to users?
I still think this PR as is provides a lot of values for users. Perhaps once this gets merged, we could also update documentation on how this can be used to provide users with a safe way of using JXPath on potentially untrusted inputs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] hummelm10 commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by GitBox <gi...@apache.org>.
hummelm10 commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1323777260
> OK, just saw that CVE-2022-41852 is marked as REJECTED: https://nvd.nist.gov/vuln/detail/CVE-2022-41852
>
>
I just saw this too. I disagree with this and think it should have remained DISPUTED but I'm not the CNA. At a minimum the documentation should be updated to highlight the risks of using this library and include that it should not accept any untrusted input. I think it's irresponsible to argue the CVE to be REJECTED without first updating the documentation since new users of the library may not be aware of this issue now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] stephanborn commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by GitBox <gi...@apache.org>.
stephanborn commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1323764579
OK, just saw that CVE-2022-41852 is marked as REJECTED: https://nvd.nist.gov/vuln/detail/CVE-2022-41852
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by "kyakdan (via GitHub)" <gi...@apache.org>.
kyakdan commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1427355816
@garydgregory @markt-asf Is there any update on when this PR can be merged?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] schlm3 commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by "schlm3 (via GitHub)" <gi...@apache.org>.
schlm3 commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1473547243
Like most of the JXPath users community, I also think that this is a security issue because of JXPath's default configuration.
However, it seems that the mitigation approach taken by @kyakdan will not make it into the official code.
The question for me now is, how all the usages of JXPath out there could be secured without having the project responsibles to change code in the project. But for all those concerned about having that security issue in their apps, there is (as far as I could find out) a secure way to get rid of the problem. You need to set the default "Functions" implementation to YOUR default instead of silently using the default from JXPathContext (which is JXPathContext.GENERIC_FUNCTIONS ). All you have to do is adding the following line for each instance of JXPathContext in your Application:
`context.setFunctions(new org.apache.commons.jxpath.FunctionLibrary());`
Of course, you will loose any way of using direct Java Class functions. But the approach also offers the possibility to add your own, secured implementation of the JXPath's PackageFunction, which could be one with a "whitelist" approach like proposed in this thread.
Many usages of JXPath however won't even require the Package Functions if they only need to traverse graphs of objects.
You could even provide your own JXPathContextFactory to your Application VM such that any Usage of JXPathContext.newContext() will return a secured context by default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999896659
##########
src/main/java/org/apache/commons/jxpath/ClassFunctions.java:
##########
@@ -91,6 +92,13 @@ public Function getFunction(
final String namespace,
final String name,
Object[] parameters) {
+ JXPathFilter jxPathFilter = new JXPathFilter();
Review Comment:
Not sure, but I am a little bit worried that this can be quite limiting to developers. We force them to use System.setProperty in order to allow some classes in jxpath functions. Also, it is not possible to change the filter per JXContext
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999896659
##########
src/main/java/org/apache/commons/jxpath/ClassFunctions.java:
##########
@@ -91,6 +92,13 @@ public Function getFunction(
final String namespace,
final String name,
Object[] parameters) {
+ JXPathFilter jxPathFilter = new JXPathFilter();
Review Comment:
Not sure, but I am a little bit worried that this can be quite limiting to developers. We force them to use system properties in order to allow some classes in jxpath functions. Also, it is not possible to change the filter per JXContext
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1001588620
##########
src/main/java/org/apache/commons/jxpath/ClassFunctions.java:
##########
@@ -91,6 +92,13 @@ public Function getFunction(
final String namespace,
final String name,
Object[] parameters) {
+ JXPathFilter jxPathFilter = new JXPathFilter();
Review Comment:
Thanks for the proposal. I've added an interface `JXPathFilter` and an implementation `SystemPropertyJXPathFilter` based on the system property of the allow list. Moreover, I have added a new default method `getFunction` to the `Functions` interface that takes a filter as a parameter. The default implementation simply calls the `getFunction` without the filter and can be overridden by implemented classes. This guarantees binary compatibility as @garydgregory pointer out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] garydgregory commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r999316059
##########
src/main/java/org/apache/commons/jxpath/Functions.java:
##########
@@ -43,4 +45,15 @@ public interface Functions {
* @return Function
*/
Function getFunction(String namespace, String name, Object[] parameters);
+
+ /**
+ * Returns a Function, if any, for the specified namespace,
+ * name and parameter types.
+ * @param namespace ns
+ * @param name function name
+ * @param parameters Object[]
+ * @param jxPathFilter the XPath filter
+ * @return Function
+ */
+ Function getFunction(String namespace, String name, Object[] parameters, JXPathFilter jxPathFilter);
Review Comment:
This needs to be a default method to avoid breaking binary compatibility. This might be a misconfiguration in our parent POM or a bug in japicmp...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] Warxim commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
Warxim commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1004641669
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -46,9 +46,11 @@ public class ExtensionFunctionTest extends JXPathTestCase {
private JXPathContext context;
private TestBean testBean;
private TypeConverter typeConverter;
+ private final String DEFAULT_ALLOW_LIST = "org.w3c.*,org.jdom.*,java.lang.String,java.util.*,org.apache.commons.*";
Review Comment:
I am also not an expert on JXPath, but I agree that it would be great to have some secure default allow list. đź‘Ť
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on a diff in pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on code in PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#discussion_r1005164446
##########
src/test/java/org/apache/commons/jxpath/ri/compiler/ExtensionFunctionTest.java:
##########
@@ -46,9 +46,11 @@ public class ExtensionFunctionTest extends JXPathTestCase {
private JXPathContext context;
private TestBean testBean;
private TypeConverter typeConverter;
+ private final String DEFAULT_ALLOW_LIST = "org.w3c.*,org.jdom.*,java.lang.String,java.util.*,org.apache.commons.*";
Review Comment:
@garydgregory Is there a default secure alow list that you can provide?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] kyakdan commented on pull request #26: Fix CVE-2022-41852
Posted by GitBox <gi...@apache.org>.
kyakdan commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1299950916
@garydgregory Thanks in advance for the feedback. Any estimation of when you would be able to have a look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] iamamoose commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by GitBox <gi...@apache.org>.
iamamoose commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1319955459
> > The CVE record has been updated to invalid so my request to edit the title of this PR to remove the CVE reference stands.
>
> For clarity, this is not true. The record has been marked DISPUTED which is entirely different to REJECT per the CVE website. (https://www.cve.org/ResourcesSupport/FAQs) Disputed is when there is disagreement between a vendor and security researcher so CVE Program leaves the record as is and updates the status. Rejected is when the record was placed in error and is not a vulnerability and the record could potentially be returned to the reserved pool in the future. The CVE is currently still valid and tied to the vulnerability just with the updated status so people can further research the vulnerability themselves.
DISPUTED is the wrong state for these issues, then, they should be REJECT as they were placed in error, are not a vulnerability (as well as against CNA rules at the time of assignment).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] ecki commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by GitBox <gi...@apache.org>.
ecki commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1325129178
Agreed, it needs to be documented. Either as a warning that it is not possible to have untrusted expressions or with a pointer to the filter mechanism discussed here. I will propose something.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] ecki commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by GitBox <gi...@apache.org>.
ecki commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1326434467
Any comment on my PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-jxpath] hummelm10 commented on pull request #26: Add an allow list for classes that can be loaded by JXPath
Posted by GitBox <gi...@apache.org>.
hummelm10 commented on PR #26:
URL: https://github.com/apache/commons-jxpath/pull/26#issuecomment-1320280537
> > > The CVE record has been updated to invalid so my request to edit the title of this PR to remove the CVE reference stands.
>
> >
>
> > For clarity, this is not true. The record has been marked DISPUTED which is entirely different to REJECT per the CVE website. (https://www.cve.org/ResourcesSupport/FAQs) Disputed is when there is disagreement between a vendor and security researcher so CVE Program leaves the record as is and updates the status. Rejected is when the record was placed in error and is not a vulnerability and the record could potentially be returned to the reserved pool in the future. The CVE is currently still valid and tied to the vulnerability just with the updated status so people can further research the vulnerability themselves.
>
>
>
> DISPUTED is the wrong state for these issues, then, they should be REJECT as they were placed in error, are not a vulnerability (as well as against CNA rules at the time of assignment).
DISPUTED is the correct state since there is a dispute between the researcher and the maintainer. They CVE was not placed in error since obviously the researcher and others here do consider it a vulnerability. The DISPUTED tag tells individuals to research the issue; which they should do because this is an issue that could allow RCE in an application.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org