hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-29 10:45:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #6136 from smowton/smowton/admin/spring-xss-content-type-sensitivity

Browse Source 
Spring HTTP: improve content-type sensitivity
This commit is contained in:
Anders Schack-Mulligen
2021-09-15 09:50:56 +02:00
committed by GitHub
parent 2a9e3da24f 406466de9a
commit 3f7d6e6f85
5 changed files with 219 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
java/ql/lib/semmle/code/java/frameworks/spring/SpringController.qll
View File

@@ -100,11 +100,23 @@ class SpringResponseBodyAnnotationType extends AnnotationType {
}
}
private class SpringRequestMappingAnnotation extends Annotation {
SpringRequestMappingAnnotation() { this.getType() instanceof SpringRequestMappingAnnotationType }
}
private Expr getProducesExpr(RefType rt) {
result = rt.getAnAnnotation().(SpringRequestMappingAnnotation).getValue("produces")
or
rt.getAnAnnotation().(SpringRequestMappingAnnotation).getValue("produces").(ArrayInit).getSize() =
0 and
result = getProducesExpr(rt.getASupertype())
}
/**
* A method on a Spring controller that is executed in response to a web request.
*/
class SpringRequestMappingMethod extends SpringControllerMethod {
Annotation requestMappingAnnotation;
SpringRequestMappingAnnotation requestMappingAnnotation;
SpringRequestMappingMethod() {
// Any method that declares the @RequestMapping annotation, or overrides a method that declares
@@ -112,18 +124,31 @@ class SpringRequestMappingMethod extends SpringControllerMethod {
// not declared with @Inherited.
exists(Method superMethod |
this.overrides*(superMethod) and
requestMappingAnnotation = superMethod.getAnAnnotation() and
requestMappingAnnotation.getType() instanceof SpringRequestMappingAnnotationType
requestMappingAnnotation = superMethod.getAnAnnotation()
)
}
/** Gets a request mapping parameter. */
SpringRequestMappingParameter getARequestParameter() { result = getAParameter() }
/** Gets the "produces" @RequestMapping annotation value, if present. */
/** Gets the "produces" @RequestMapping annotation value, if present. If an array is specified, gets the array. */
Expr getProducesExpr() {
result = requestMappingAnnotation.getValue("produces")
or
requestMappingAnnotation.getValue("produces").(ArrayInit).getSize() = 0 and
result = getProducesExpr(this.getDeclaringType())
}
/** Gets a "produces" @RequestMapping annotation value. If an array is specified, gets a member of the array. */
Expr getAProducesExpr() {
result = this.getProducesExpr() and not result instanceof ArrayInit
or
result = this.getProducesExpr().(ArrayInit).getAnInit()
}
/** Gets the "produces" @RequestMapping annotation value, if present and a string constant. */
string getProduces() {
result =
requestMappingAnnotation.getValue("produces").(CompileTimeConstantExpr).getStringValue()
result = this.getProducesExpr().(CompileTimeConstantExpr).getStringValue()
}
/** Gets the "value" @RequestMapping annotation value, if present. */

178
java/ql/lib/semmle/code/java/frameworks/spring/SpringHttp.qll
View File

@@ -5,6 +5,9 @@
import java
private import semmle.code.java.dataflow.ExternalFlow
private import semmle.code.java.dataflow.DataFlow
private import semmle.code.java.frameworks.spring.SpringController
private import semmle.code.java.security.XSS as XSS
/** The class `org.springframework.http.HttpEntity` or an instantiation of it. */
class SpringHttpEntity extends Class {
@@ -140,3 +143,178 @@ private class SpringHttpFlowStep extends SummaryModelCsv {
]
}
}
private predicate specifiesContentType(SpringRequestMappingMethod method) {
exists(method.getAProducesExpr())
}
private class SpringXssSink extends XSS::XssSink {
SpringXssSink() {
exists(SpringRequestMappingMethod requestMappingMethod, ReturnStmt rs |
requestMappingMethod = rs.getEnclosingCallable() and
this.asExpr() = rs.getResult() and
(
not specifiesContentType(requestMappingMethod) or
isXssVulnerableContentTypeExpr(requestMappingMethod.getAProducesExpr())
)
|
// If a Spring request mapping method is either annotated with @ResponseBody (or equivalent),
// or returns a HttpEntity or sub-type, then the return value of the method is converted into
// a HTTP reponse using a HttpMessageConverter implementation. The implementation is chosen
// based on the return type of the method, and the Accept header of the request.
//
// By default, the only message converter which produces a response which is vulnerable to
// XSS is the StringHttpMessageConverter, which "Accept"s all text/* content types, including
// text/html. Therefore, if a browser request includes "text/html" in the "Accept" header,
// any String returned will be converted into a text/html response.
requestMappingMethod.isResponseBody() and
requestMappingMethod.getReturnType() instanceof TypeString
or
exists(Type returnType |
// A return type of HttpEntity<T> or ResponseEntity<T> represents an HTTP response with both
// a body and a set of headers. The body is subject to the same HttpMessageConverter
// process as above.
returnType = requestMappingMethod.getReturnType() and
(
returnType instanceof SpringHttpEntity
or
returnType instanceof SpringResponseEntity
)
|
// The type argument, representing the type of the body, is type String
returnType.(ParameterizedClass).getTypeArgument(0) instanceof TypeString
or
// Return type is a Raw class, which means no static type information on the body. In this
// case we will still treat this as an XSS sink, but rely on our taint flow steps for
// HttpEntity/ResponseEntity to only pass taint into those instances if the body type was
// String.
returnType instanceof RawClass
)
)
}
}
private string getSpringConstantContentType(FieldAccess e) {
e.getQualifier().getType().(RefType).hasQualifiedName("org.springframework.http", "MediaType") and
exists(string fieldName | e.getField().hasName(fieldName) |
fieldName = "APPLICATION_ATOM_XML" + ["", "_VALUE"] and result = "application/atom+xml"
or
fieldName = "APPLICATION_CBOR" + ["", "_VALUE"] and result = "application/cbor"
or
fieldName = "APPLICATION_FORM_URLENCODED" + ["", "_VALUE"] and
result = "application/x-www-form-urlencoded"
or
fieldName = "APPLICATION_JSON" + ["", "_VALUE"] and result = "application/json"
or
fieldName = "APPLICATION_JSON_UTF8" + ["", "_VALUE"] and
result = "application/json;charset=UTF-8"
or
fieldName = "APPLICATION_NDJSON" + ["", "_VALUE"] and result = "application/x-ndjson"
or
fieldName = "APPLICATION_OCTET_STREAM" + ["", "_VALUE"] and result = "application/octet-stream"
or
fieldName = "APPLICATION_PDF" + ["", "_VALUE"] and result = "application/pdf"
or
fieldName = "APPLICATION_PROBLEM_JSON" + ["", "_VALUE"] and result = "application/problem+json"
or
fieldName = "APPLICATION_PROBLEM_JSON_UTF8" + ["", "_VALUE"] and
result = "application/problem+json;charset=UTF-8"
or
fieldName = "APPLICATION_PROBLEM_XML" + ["", "_VALUE"] and result = "application/problem+xml"
or
fieldName = "APPLICATION_RSS_XML" + ["", "_VALUE"] and result = "application/rss+xml"
or
fieldName = "APPLICATION_STREAM_JSON" + ["", "_VALUE"] and result = "application/stream+json"
or
fieldName = "APPLICATION_XHTML_XML" + ["", "_VALUE"] and result = "application/xhtml+xml"
or
fieldName = "APPLICATION_XML" + ["", "_VALUE"] and result = "application/xml"
or
fieldName = "IMAGE_GIF" + ["", "_VALUE"] and result = "image/gif"
or
fieldName = "IMAGE_JPEG" + ["", "_VALUE"] and result = "image/jpeg"
or
fieldName = "IMAGE_PNG" + ["", "_VALUE"] and result = "image/png"
or
fieldName = "MULTIPART_FORM_DATA" + ["", "_VALUE"] and result = "multipart/form-data"
or
fieldName = "MULTIPART_MIXED" + ["", "_VALUE"] and result = "multipart/mixed"
or
fieldName = "MULTIPART_RELATED" + ["", "_VALUE"] and result = "multipart/related"
or
fieldName = "TEXT_EVENT_STREAM" + ["", "_VALUE"] and result = "text/event-stream"
or
fieldName = "TEXT_HTML" + ["", "_VALUE"] and result = "text/html"
or
fieldName = "TEXT_MARKDOWN" + ["", "_VALUE"] and result = "text/markdown"
or
fieldName = "TEXT_PLAIN" + ["", "_VALUE"] and result = "text/plain"
or
fieldName = "TEXT_XML" + ["", "_VALUE"] and result = "text/xml"
)
}
private predicate isXssVulnerableContentTypeExpr(Expr e) {
XSS::isXssVulnerableContentType(e.(CompileTimeConstantExpr).getStringValue()) or
XSS::isXssVulnerableContentType(getSpringConstantContentType(e))
}
private predicate isXssSafeContentTypeExpr(Expr e) {
XSS::isXssSafeContentType(e.(CompileTimeConstantExpr).getStringValue()) or
XSS::isXssSafeContentType(getSpringConstantContentType(e))
}
private DataFlow::Node getABodyBuilderWithExplicitContentType(Expr contentType) {
result.asExpr() =
any(MethodAccess ma |
ma.getCallee()
.hasQualifiedName("org.springframework.http", "ResponseEntity$BodyBuilder", "contentType") and
contentType = ma.getArgument(0)
)
or
result.asExpr() =
any(MethodAccess ma |
ma.getQualifier() = getABodyBuilderWithExplicitContentType(contentType).asExpr() and
ma.getType()
.(RefType)
.hasQualifiedName("org.springframework.http", "ResponseEntity$BodyBuilder")
)
or
DataFlow::localFlow(getABodyBuilderWithExplicitContentType(contentType), result)
}
private DataFlow::Node getASanitizedBodyBuilder() {
result = getABodyBuilderWithExplicitContentType(any(Expr e | isXssSafeContentTypeExpr(e)))
}
private DataFlow::Node getAVulnerableBodyBuilder() {
result = getABodyBuilderWithExplicitContentType(any(Expr e | isXssVulnerableContentTypeExpr(e)))
}
private class SanitizedBodyCall extends XSS::XssSanitizer {
SanitizedBodyCall() {
this.asExpr() =
any(MethodAccess ma |
ma.getQualifier() = getASanitizedBodyBuilder().asExpr() and
ma.getCallee().hasName("body")
).getArgument(0)
}
}
/**
* Mark BodyBuilder.body calls with an explicitly vulnerable Content-Type as themselves sinks,
* as the eventual return site from a RequestHandler may have a benign @Produces annotation that
* would otherwise sanitise the result.
*
* Note these are SinkBarriers so that a return from a RequestHandlerMethod is not also flagged
* for the same path.
*/
private class ExplicitlyVulnerableBodyArgument extends XSS::XssSinkBarrier {
ExplicitlyVulnerableBodyArgument() {
this.asExpr() =
any(MethodAccess ma |
ma.getQualifier() = getAVulnerableBodyBuilder().asExpr() and
ma.getCallee().hasName("body")
).getArgument(0)
}
}

42
java/ql/lib/semmle/code/java/security/XSS.qll
View File

@@ -46,48 +46,6 @@ private class DefaultXssSink extends XssSink {
writer.hasFlowToExpr(ma.getQualifier()) and
this.asExpr() = ma.getArgument(_)
)
or
exists(SpringRequestMappingMethod requestMappingMethod, ReturnStmt rs |
requestMappingMethod = rs.getEnclosingCallable() and
this.asExpr() = rs.getResult() and
(
not exists(requestMappingMethod.getProduces()) or
requestMappingMethod.getProduces().matches("text/%")
)
|
// If a Spring request mapping method is either annotated with @ResponseBody (or equivalent),
// or returns a HttpEntity or sub-type, then the return value of the method is converted into
// a HTTP reponse using a HttpMessageConverter implementation. The implementation is chosen
// based on the return type of the method, and the Accept header of the request.
//
// By default, the only message converter which produces a response which is vulnerable to
// XSS is the StringHttpMessageConverter, which "Accept"s all text/* content types, including
// text/html. Therefore, if a browser request includes "text/html" in the "Accept" header,
// any String returned will be converted into a text/html response.
requestMappingMethod.isResponseBody() and
requestMappingMethod.getReturnType() instanceof TypeString
or
exists(Type returnType |
// A return type of HttpEntity<T> or ResponseEntity<T> represents an HTTP response with both
// a body and a set of headers. The body is subject to the same HttpMessageConverter
// process as above.
returnType = requestMappingMethod.getReturnType() and
(
returnType instanceof SpringHttpEntity
or
returnType instanceof SpringResponseEntity
)
|
// The type argument, representing the type of the body, is type String
returnType.(ParameterizedClass).getTypeArgument(0) instanceof TypeString
or
// Return type is a Raw class, which means no static type information on the body. In this
// case we will still treat this as an XSS sink, but rely on our taint flow steps for
// HttpEntity/ResponseEntity to only pass taint into those instances if the body type was
// String.
returnType instanceof RawClass
)
)
}
}

16
java/ql/test/query-tests/security/CWE-079/semmle/tests/SpringXSS.java
View File

@@ -28,11 +28,11 @@ public class SpringXSS {
}
else {
if(chainDirectly) {
return builder.contentType(MediaType.APPLICATION_JSON).body(userControlled); // $SPURIOUS: xss
return builder.contentType(MediaType.APPLICATION_JSON).body(userControlled);
}
else {
ResponseEntity.BodyBuilder builder2 = builder.contentType(MediaType.APPLICATION_JSON);
return builder2.body(userControlled); // $SPURIOUS: xss
return builder2.body(userControlled);
}
}
@@ -60,7 +60,7 @@ public class SpringXSS {
@GetMapping(value = "/xyz", produces = MediaType.TEXT_HTML_VALUE)
public static ResponseEntity<String> methodContentTypeUnsafe(String userControlled) {
return ResponseEntity.ok(userControlled); // $MISSING: xss
return ResponseEntity.ok(userControlled); // $xss
}
@GetMapping(value = "/xyz", produces = "text/html")
@@ -75,7 +75,7 @@ public class SpringXSS {
@GetMapping(value = "/xyz", produces = MediaType.APPLICATION_JSON_VALUE)
public static ResponseEntity<String> methodContentTypeSafeOverriddenWithUnsafe(String userControlled) {
return ResponseEntity.ok().contentType(MediaType.TEXT_HTML).body(userControlled); // $MISSING: xss
return ResponseEntity.ok().contentType(MediaType.TEXT_HTML).body(userControlled); // $xss
}
@GetMapping(value = "/xyz", produces = MediaType.TEXT_HTML_VALUE)
@@ -105,12 +105,12 @@ public class SpringXSS {
private static class ClassContentTypeSafe {
@GetMapping(value = "/abc")
public ResponseEntity<String> test(String userControlled) {
return ResponseEntity.ok(userControlled); // $SPURIOUS: xss
return ResponseEntity.ok(userControlled);
}
@GetMapping(value = "/abc")
public String testDirectReturn(String userControlled) {
return userControlled; // $SPURIOUS: xss
return userControlled;
}
@GetMapping(value = "/xyz", produces = {"text/html"})
@@ -139,12 +139,12 @@ public class SpringXSS {
@GetMapping(value = "/xyz", produces = {"application/json"})
public ResponseEntity<String> overridesWithSafe(String userControlled) {
return ResponseEntity.ok(userControlled); // $SPURIOUS: xss
return ResponseEntity.ok(userControlled);
}
@GetMapping(value = "/abc")
public ResponseEntity<String> overridesWithSafe2(String userControlled) {
return ResponseEntity.ok().contentType(MediaType.APPLICATION_JSON).body(userControlled); // $SPURIOUS: xss
return ResponseEntity.ok().contentType(MediaType.APPLICATION_JSON).body(userControlled);
}
}