From 43b9199734be4f0fba60ad095e638c843a7f696d Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 7 Aug 2023 10:21:58 +0200 Subject: [PATCH 1/2] Java: Improved JaxWsEndpoint::getARemoteMethod --- ...2023-08-07-jaxrs-webmethod-improvements.md | 4 + .../lib/semmle/code/java/frameworks/JaxWS.qll | 105 +++++++++++++++++- .../frameworks/JaxWs/JaxWsEndpoint.java | 54 +++++++++ .../adapters/XmlJavaTypeAdapter.java | 15 +++ 4 files changed, 174 insertions(+), 4 deletions(-) create mode 100644 java/ql/lib/change-notes/2023-08-07-jaxrs-webmethod-improvements.md create mode 100644 java/ql/test/stubs/jaxws-api-2.0/javax/xml/bind/annotation/adapters/XmlJavaTypeAdapter.java diff --git a/java/ql/lib/change-notes/2023-08-07-jaxrs-webmethod-improvements.md b/java/ql/lib/change-notes/2023-08-07-jaxrs-webmethod-improvements.md new file mode 100644 index 00000000000..be19599c865 --- /dev/null +++ b/java/ql/lib/change-notes/2023-08-07-jaxrs-webmethod-improvements.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The predicate `JaxWsEndpoint::getARemoteMethod` no longer requires the result to be annotated with `@WebMethod`. Instead, the requirements listed in the JAX-RPC Specification 1.1 for required parameter and return types are used. Applications using JAX-RS may see an increase in results. diff --git a/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll b/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll index 54b41f28a08..3f759a0f697 100644 --- a/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll +++ b/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll @@ -4,6 +4,8 @@ */ import java +private import semmle.code.java.frameworks.Networking +private import semmle.code.java.frameworks.Rmi private import semmle.code.java.security.XSS /** @@ -28,11 +30,106 @@ class JaxWsEndpoint extends Class { ) } - /** Gets a method annotated with `@WebMethod` or `@WebEndpoint`. */ - Callable getARemoteMethod() { + /** + * Gets a method of this class that is not an excluded `@WebMethod`, + * and the parameters and return value of which are either of an acceptable type, + * or are annotated with `@XmlJavaTypeAdapter`. + */ + Method getARemoteMethod() { result = this.getACallable() and - exists(AnnotationType a | a = result.getAnAnnotation().getType() | - a.hasName(["WebMethod", "WebEndpoint"]) + not result instanceof InitializerMethod and + not exists(Annotation a | a = result.getAnAnnotation() | + a.getType().hasQualifiedName(["javax", "jakarta"] + ".jws", "WebMethod") and + a.getValue("exclude").(BooleanLiteral).getBooleanValue() = true + ) and + forex(ParamOrReturn paramOrRet | paramOrRet = result.getAParameter() or paramOrRet = result | + exists(Type t | t = paramOrRet.getType() | + t instanceof JaxAcceptableType + or + t.(Annotatable).getAnAnnotation().getType() instanceof XmlJavaTypeAdapter + or + t instanceof VoidType + ) + or + paramOrRet.getInheritedAnnotation().getType() instanceof XmlJavaTypeAdapter + ) + } +} + +/** The annotation type `@XmlJavaTypeAdapter`. */ +class XmlJavaTypeAdapter extends AnnotationType { + XmlJavaTypeAdapter() { + this.hasQualifiedName(["javax", "jakarta"] + ".xml.bind.annotation.adapters", + "XmlJavaTypeAdapter") + } +} + +private class ParamOrReturn extends Annotatable { + ParamOrReturn() { this instanceof Parameter or this instanceof Method } + + Type getType() { + result = this.(Parameter).getType() + or + result = this.(Method).getReturnType() + } + + Annotation getInheritedAnnotation() { + result = this.getAnAnnotation() + or + result = this.(Method).getAnOverride*().getAnAnnotation() + or + result = + this.(Parameter) + .getCallable() + .(Method) + .getAnOverride*() + .getParameter(this.(Parameter).getPosition()) + .getAnAnnotation() + } +} + +// JAX-RPC 1.1, section 5 +private class JaxAcceptableType extends Type { + JaxAcceptableType() { + // JAX-RPC 1.1, section 5.1.1 + this instanceof PrimitiveType + or + // JAX-RPC 1.1, section 5.1.2 + this.(Array).getElementType() instanceof JaxAcceptableType + or + // JAX-RPC 1.1, section 5.1.3 + this instanceof JaxAcceptableStandardClass + or + // JAX-RPC 1.1, section 5.1.4 + this instanceof JaxValueType + } +} + +private class JaxAcceptableStandardClass extends RefType { + JaxAcceptableStandardClass() { + this instanceof TypeString or + this.hasQualifiedName("java.util", "Date") or + this.hasQualifiedName("java.util", "Calendar") or + this.hasQualifiedName("java.math", "BigInteger") or + this.hasQualifiedName("java.math", "BigDecimal") or + this.hasQualifiedName("javax.xml.namespace", "QName") or + this instanceof TypeUri + } +} + +// JAX-RPC 1.1, section 5.4 +private class JaxValueType extends RefType { + JaxValueType() { + not this instanceof Wildcard and + // Mutually exclusive with other `JaxAcceptableType`s + not this instanceof Array and + not this instanceof JaxAcceptableStandardClass and + not this.getPackage().getName().matches("java.%") and + // Must not implement (directly or indirectly) the java.rmi.Remote interface. + not this.getAnAncestor() instanceof TypeRemote and + // The Java type of a public field must be a supported JAX-RPC type as specified in the section 5.1. + forall(Field f | this.getAMember() = f and f.isPublic() | + f.getType() instanceof JaxAcceptableType ) } } diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JaxWsEndpoint.java b/java/ql/test/library-tests/frameworks/JaxWs/JaxWsEndpoint.java index 511508cd774..6118f88852b 100644 --- a/java/ql/test/library-tests/frameworks/JaxWs/JaxWsEndpoint.java +++ b/java/ql/test/library-tests/frameworks/JaxWs/JaxWsEndpoint.java @@ -1,5 +1,8 @@ +import java.io.File; + import javax.jws.WebMethod; import javax.jws.WebService; +import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter; import javax.xml.ws.WebEndpoint; import javax.xml.ws.WebServiceClient; import javax.xml.ws.WebServiceProvider; @@ -15,8 +18,25 @@ class WebServiceClass { // $ JaxWsEndpoint void WebEndpointMethod() { // $ JaxWsEndpointRemoteMethod } + String acceptableTypes(String param) { // $ JaxWsEndpointRemoteMethod + return null; + } + + String unacceptableParamType(File param) { // not an endpoint + return null; + } + + File unacceptableReturnType() { // not an endpoint + return null; + } + + @XmlJavaTypeAdapter + File annotatedTypes(@XmlJavaTypeAdapter File param) { // $ JaxWsEndpointRemoteMethod + return null; + } } + @WebServiceProvider class WebServiceProviderClass { // $ JaxWsEndpoint @@ -28,8 +48,25 @@ class WebServiceProviderClass { // $ JaxWsEndpoint void WebEndpointMethod() { // $ JaxWsEndpointRemoteMethod } + String acceptableTypes(String param) { // $ JaxWsEndpointRemoteMethod + return null; + } + + String unacceptableParamType(File param) { // not an endpoint + return null; + } + + File unacceptableReturnType() { // not an endpoint + return null; + } + + @XmlJavaTypeAdapter + File annotatedTypes(@XmlJavaTypeAdapter File param) { // $ JaxWsEndpointRemoteMethod + return null; + } } + @WebServiceClient class WebServiceClientClass { // $ JaxWsEndpoint @@ -41,4 +78,21 @@ class WebServiceClientClass { // $ JaxWsEndpoint void WebEndpointMethod() { // $ JaxWsEndpointRemoteMethod } + String acceptableTypes(String param) { // $ JaxWsEndpointRemoteMethod + return null; + } + + String unacceptableParamType(File param) { // not an endpoint + return null; + } + + File unacceptableReturnType() { // not an endpoint + return null; + } + + @XmlJavaTypeAdapter + File annotatedTypes(@XmlJavaTypeAdapter File param) { // $ JaxWsEndpointRemoteMethod + return null; + } + } diff --git a/java/ql/test/stubs/jaxws-api-2.0/javax/xml/bind/annotation/adapters/XmlJavaTypeAdapter.java b/java/ql/test/stubs/jaxws-api-2.0/javax/xml/bind/annotation/adapters/XmlJavaTypeAdapter.java new file mode 100644 index 00000000000..467b99c5470 --- /dev/null +++ b/java/ql/test/stubs/jaxws-api-2.0/javax/xml/bind/annotation/adapters/XmlJavaTypeAdapter.java @@ -0,0 +1,15 @@ +package javax.xml.bind.annotation.adapters; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.TYPE; +import static java.lang.annotation.ElementType.PARAMETER; +import static java.lang.annotation.ElementType.PACKAGE; + +@Retention(RUNTIME) +@Target({PACKAGE, FIELD, METHOD, TYPE, PARAMETER}) +public @interface XmlJavaTypeAdapter { +} From 3f9701cea7bc9901f36714bac626acb4eeacdcc2 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Thu, 24 Aug 2023 11:35:52 +0200 Subject: [PATCH 2/2] Two fixes: * Consider that the @WebService annotation (et al) can be in a supertype or interface * getARemoteMethod should only return public methods, since protected, package-private, and private methods are not exposed --- .../lib/semmle/code/java/frameworks/JaxWS.qll | 3 +- .../frameworks/JaxWs/JaxWsEndpoint.java | 36 +++++++++---------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll b/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll index 3f759a0f697..18a5d800df5 100644 --- a/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll +++ b/java/ql/lib/semmle/code/java/frameworks/JaxWS.qll @@ -25,7 +25,7 @@ string getAJaxRsPackage(string subpackage) { result = getAJaxRsPackage() + "." + */ class JaxWsEndpoint extends Class { JaxWsEndpoint() { - exists(AnnotationType a | a = this.getAnAnnotation().getType() | + exists(AnnotationType a | a = this.getAnAncestor().getAnAnnotation().getType() | a.hasName(["WebService", "WebServiceProvider", "WebServiceClient"]) ) } @@ -37,6 +37,7 @@ class JaxWsEndpoint extends Class { */ Method getARemoteMethod() { result = this.getACallable() and + result.isPublic() and not result instanceof InitializerMethod and not exists(Annotation a | a = result.getAnAnnotation() | a.getType().hasQualifiedName(["javax", "jakarta"] + ".jws", "WebMethod") and diff --git a/java/ql/test/library-tests/frameworks/JaxWs/JaxWsEndpoint.java b/java/ql/test/library-tests/frameworks/JaxWs/JaxWsEndpoint.java index 6118f88852b..b3ca779d345 100644 --- a/java/ql/test/library-tests/frameworks/JaxWs/JaxWsEndpoint.java +++ b/java/ql/test/library-tests/frameworks/JaxWs/JaxWsEndpoint.java @@ -11,27 +11,27 @@ import javax.xml.ws.WebServiceProvider; class WebServiceClass { // $ JaxWsEndpoint @WebMethod - void WebMethodMethod() { // $ JaxWsEndpointRemoteMethod + public void WebMethodMethod() { // $ JaxWsEndpointRemoteMethod } @WebEndpoint - void WebEndpointMethod() { // $ JaxWsEndpointRemoteMethod + public void WebEndpointMethod() { // $ JaxWsEndpointRemoteMethod } - String acceptableTypes(String param) { // $ JaxWsEndpointRemoteMethod + public String acceptableTypes(String param) { // $ JaxWsEndpointRemoteMethod return null; } - String unacceptableParamType(File param) { // not an endpoint + public String unacceptableParamType(File param) { // not an endpoint return null; } - File unacceptableReturnType() { // not an endpoint + public File unacceptableReturnType() { // not an endpoint return null; } @XmlJavaTypeAdapter - File annotatedTypes(@XmlJavaTypeAdapter File param) { // $ JaxWsEndpointRemoteMethod + public File annotatedTypes(@XmlJavaTypeAdapter File param) { // $ JaxWsEndpointRemoteMethod return null; } } @@ -41,27 +41,27 @@ class WebServiceClass { // $ JaxWsEndpoint class WebServiceProviderClass { // $ JaxWsEndpoint @WebMethod - void WebMethodMethod() { // $ JaxWsEndpointRemoteMethod + public void WebMethodMethod() { // $ JaxWsEndpointRemoteMethod } @WebEndpoint - void WebEndpointMethod() { // $ JaxWsEndpointRemoteMethod + public void WebEndpointMethod() { // $ JaxWsEndpointRemoteMethod } - String acceptableTypes(String param) { // $ JaxWsEndpointRemoteMethod + public String acceptableTypes(String param) { // $ JaxWsEndpointRemoteMethod return null; } - String unacceptableParamType(File param) { // not an endpoint + public String unacceptableParamType(File param) { // not an endpoint return null; } - File unacceptableReturnType() { // not an endpoint + public File unacceptableReturnType() { // not an endpoint return null; } @XmlJavaTypeAdapter - File annotatedTypes(@XmlJavaTypeAdapter File param) { // $ JaxWsEndpointRemoteMethod + public File annotatedTypes(@XmlJavaTypeAdapter File param) { // $ JaxWsEndpointRemoteMethod return null; } } @@ -71,27 +71,27 @@ class WebServiceProviderClass { // $ JaxWsEndpoint class WebServiceClientClass { // $ JaxWsEndpoint @WebMethod - void WebMethodMethod() { // $ JaxWsEndpointRemoteMethod + public void WebMethodMethod() { // $ JaxWsEndpointRemoteMethod } @WebEndpoint - void WebEndpointMethod() { // $ JaxWsEndpointRemoteMethod + public void WebEndpointMethod() { // $ JaxWsEndpointRemoteMethod } - String acceptableTypes(String param) { // $ JaxWsEndpointRemoteMethod + public String acceptableTypes(String param) { // $ JaxWsEndpointRemoteMethod return null; } - String unacceptableParamType(File param) { // not an endpoint + public String unacceptableParamType(File param) { // not an endpoint return null; } - File unacceptableReturnType() { // not an endpoint + public File unacceptableReturnType() { // not an endpoint return null; } @XmlJavaTypeAdapter - File annotatedTypes(@XmlJavaTypeAdapter File param) { // $ JaxWsEndpointRemoteMethod + public File annotatedTypes(@XmlJavaTypeAdapter File param) { // $ JaxWsEndpointRemoteMethod return null; }