isudana commented on code in PR #124:
URL: https://github.com/apache/synapse/pull/124#discussion_r4534050887
##########
modules/core/src/main/java/org/apache/synapse/mediators/Value.java:
##########
@@ -195,23 +218,90 @@ public boolean hasPropertyEvaluateExpr(){
public Object evalutePropertyExpression(MessageContext synCtx) {
SynapseXPath expr = null;
+ SynapseXPath expression;
try {
expr = new SynapseXPath(this.keyValue.substring(1,
keyValue.length() - 1));
for (OMNamespace aNamespaceList : namespaceList) {
expr.addNamespace(aNamespaceList);
}
String result = expr.stringValueOf(synCtx);
- SynapseXPath expression = new SynapseXPath(result);
+ expression = new SynapseXPath(result);
for (OMNamespace aNamespaceList : namespaceList) {
expression.addNamespace(aNamespaceList);
}
- return expression;
} catch (Exception e) {
handleException("Can not evaluate escaped expression : " + expr);
+ return this.expression;
}
- return this.expression;
+ String restrictedFunction =
findRestrictedFunction(expression.getRootExpr());
+ if (restrictedFunction != null) {
+ handleException("The function '" + restrictedFunction + "' is not
allowed in a "
+ + "dynamically evaluated template parameter expression");
+ }
+ return expression;
+ }
+
+ /**
+ * Walks a parsed XPath expression tree and returns the name of the first
function call
+ * that is in {@link #RESTRICTED_DYNAMIC_FUNCTIONS}, or {@code null} if
there is none. The
+ * whole tree is inspected, including function arguments and step
predicates.
+ *
+ * @param node a node of the parsed expression tree (an {@link Expr},
{@link Step} or
+ * {@link Predicate}); may be {@code null}
+ * @return the restricted function name if one is present, otherwise
{@code null}
+ */
+ private String findRestrictedFunction(Object node) {
+ if (node == null) {
+ return null;
+ }
+ if (node instanceof FunctionCallExpr) {
Review Comment:
findRestrictedFunction only flags a function call that has no prefix, so
ns:document(...) is let through. But if a prefix isn’t bound to a namespace,
Jaxen treats it as having no namespace. It then looks the function up under the
same key as the built-in document(), and the built-in one runs.
document('file:/tmp/probe.xml') -> LEAKED-FROM-DISK
ns:document('file:/tmp/probe.xml') -> LEAKED-FROM-DISK
foo:evaluate('concat("a","b")') -> ab
So testNamespacedDocumentIsNotRestricted locks the bypass in as expected
behaviour. Its own input, ns:document("x") with ns unbound, would call the real
document().
Suggested fix: match on the function name and ignore the prefix. Synapse
already resolves its own functions, such as get-property, by name only.
```java
if (node instanceof FunctionCallExpr) {
FunctionCallExpr call = (FunctionCallExpr) node;
String name = call.getFunctionName();
if (name != null && RESTRICTED_DYNAMIC_FUNCTIONS.contains(name)) {
return name;
}
return findRestrictedFunction(call.getParameters());
}
```
Then change that test to assertRejected("ns:document(...)"). If they want to
keep allowing namespaced functions, the exemption should apply only when the
prefix actually maps to a non-empty namespace.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
