mirror of
https://github.com/github/codeql.git
synced 2026-04-27 17:55:19 +02:00
trying to hone in on eq comparison and include?
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* @name Manually checking http verb instead of using built in rails routes and protections
|
||||
* @description Manually checking HTTP verbs is an indication that multiple requests are routed to the same controller action. This could lead to bypassing necessary authorization methods and other protections, like CSRF protection. Prefer using different controller actions for each HTTP method and relying Rails routing to handle mappting resources and verbs to specific methods.
|
||||
* @description Manually checking HTTP verbs is an indication that multiple requests are routed to the same controller action. This could lead to bypassing necessary authorization methods and other protections, like CSRF protection. Prefer using different controller actions for each HTTP method and relying Rails routing to handle mappting resources and verbs to specific methods.
|
||||
* @kind problem
|
||||
* @problem.severity error
|
||||
* @security-severity 7.0
|
||||
@@ -20,59 +20,63 @@ class ManuallyCheckHttpVerb extends DataFlow::CallNode {
|
||||
this instanceof CheckPostRequest or
|
||||
this instanceof CheckDeleteRequest or
|
||||
this instanceof CheckHeadRequest or
|
||||
this.asExpr().getExpr() instanceof CheckRequestMethodFromEnv
|
||||
this instanceof CheckRequestMethodFromEnv
|
||||
}
|
||||
}
|
||||
|
||||
class CheckRequestMethodFromEnv extends ComparisonOperation {
|
||||
class CheckRequestMethodFromEnv extends DataFlow::CallNode {
|
||||
CheckRequestMethodFromEnv() {
|
||||
this.getAnOperand() instanceof GetRequestMethodFromEnv
|
||||
// is this node an instance of `env["REQUEST_METHOD"]
|
||||
this.getExprNode().getNode() instanceof GetRequestMethodFromEnv and
|
||||
(
|
||||
// and is this node a param of a call to `.include?`
|
||||
exists(MethodCall call | call.getAnArgument() = this.getExprNode().getNode() |
|
||||
call.getMethodName() = "include?"
|
||||
)
|
||||
or
|
||||
exists(DataFlow::Node node |
|
||||
node.asExpr().getExpr().(MethodCall).getMethodName() = "include?"
|
||||
|
|
||||
node.getALocalSource() = this
|
||||
)
|
||||
or
|
||||
// or is this node on either size of an equality comparison
|
||||
exists(EqualityOperation eq | eq.getAChild() = this.getExprNode().getNode())
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
class GetRequestMethodFromEnv extends ElementReference {
|
||||
GetRequestMethodFromEnv() {
|
||||
this.getAChild+().toString() = "REQUEST_METHOD" // and
|
||||
// this.getReceiver().toString() = "env"
|
||||
this.getAChild+().toString() = "REQUEST_METHOD" and
|
||||
this.getAChild+().toString() = "env"
|
||||
}
|
||||
}
|
||||
|
||||
class CheckGetRequest extends DataFlow::CallNode {
|
||||
CheckGetRequest() {
|
||||
this.getMethodName() = "get?"
|
||||
}
|
||||
CheckGetRequest() { this.getMethodName() = "get?" }
|
||||
}
|
||||
|
||||
class CheckPostRequest extends DataFlow::CallNode {
|
||||
CheckPostRequest() {
|
||||
this.getMethodName() = "post?"
|
||||
}
|
||||
CheckPostRequest() { this.getMethodName() = "post?" }
|
||||
}
|
||||
|
||||
class CheckPutRequest extends DataFlow::CallNode {
|
||||
CheckPutRequest() {
|
||||
this.getMethodName() = "put?"
|
||||
}
|
||||
CheckPutRequest() { this.getMethodName() = "put?" }
|
||||
}
|
||||
|
||||
class CheckPatchRequest extends DataFlow::CallNode {
|
||||
CheckPatchRequest() {
|
||||
this.getMethodName() = "patch?"
|
||||
}
|
||||
CheckPatchRequest() { this.getMethodName() = "patch?" }
|
||||
}
|
||||
|
||||
class CheckDeleteRequest extends DataFlow::CallNode {
|
||||
CheckDeleteRequest() {
|
||||
this.getMethodName() = "delete?"
|
||||
}
|
||||
CheckDeleteRequest() { this.getMethodName() = "delete?" }
|
||||
}
|
||||
|
||||
class CheckHeadRequest extends DataFlow::CallNode {
|
||||
CheckHeadRequest() {
|
||||
this.getMethodName() = "head?"
|
||||
}
|
||||
CheckHeadRequest() { this.getMethodName() = "head?" }
|
||||
}
|
||||
|
||||
from ManuallyCheckHttpVerb check
|
||||
where check.asExpr().getExpr().getAControlFlowNode().isCondition()
|
||||
select check, "Manually checking HTTP verbs is an indication that multiple requests are routed to the same controller action. This could lead to bypassing necessary authorization methods and other protections, like CSRF protection. Prefer using different controller actions for each HTTP method and relying Rails routing to handle mappting resources and verbs to specific methods."
|
||||
select check,
|
||||
"Manually checking HTTP verbs is an indication that multiple requests are routed to the same controller action. This could lead to bypassing necessary authorization methods and other protections, like CSRF protection. Prefer using different controller actions for each HTTP method and relying Rails routing to handle mappting resources and verbs to specific methods."
|
||||
|
||||
Reference in New Issue
Block a user