hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-27 01:35:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

attempt to introduce dataflow tracking

Browse Source
This commit is contained in:
thiggy1342
2022-06-23 02:21:47 +00:00
committed by GitHub
parent 53729f99c5
commit e838b83f5f
1 changed files with 48 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
ruby/ql/src/experimental/weak-params/WeakParams.ql
View File

@@ -1,7 +1,7 @@
/**
* @name Weak or direct parameter references are used
* @description Directly checking request parameters without following a strong params pattern can lead to unintentional avenues for injection attacks.
* @kind problem
* @kind path-problem
* @problem.severity error
* @security-severity 5.0
* @precision low
@@ -10,10 +10,13 @@
*/
import ruby
import codeql.ruby.DataFlow
import codeql.ruby.TaintTracking
import DataFlow::PathGraph
class WeakParams extends AstNode {
class WeakParams extends Expr {
WeakParams() {
this instanceof UnspecificParamsMethod or
allParamsAccess(this) or
this instanceof ParamsReference
}
}
@@ -26,27 +29,51 @@ class StrongParamsMethod extends Method {
StrongParamsMethod() { this.getName().regexpMatch(".*_params") }
}
class UnspecificParamsMethod extends MethodCall {
UnspecificParamsMethod() {
(
this.getMethodName() = "expose_all" or
this.getMethodName() = "original_hash" or
this.getMethodName() = "path_parametes" or
this.getMethodName() = "query_parameters" or
this.getMethodName() = "request_parameters" or
this.getMethodName() = "GET" or
this.getMethodName() = "POST"
)
}
predicate allParamsAccess(MethodCall call) {
call.getMethodName() = "expose_all" or
call.getMethodName() = "original_hash" or
call.getMethodName() = "path_parametes" or
call.getMethodName() = "query_parameters" or
call.getMethodName() = "request_parameters" or
call.getMethodName() = "GET" or
call.getMethodName() = "POST"
}
class ParamsReference extends ElementReference {
ParamsReference() { this.getAChild().toString() = "params" }
}
from WeakParams params
where
not params.getEnclosingMethod() instanceof StrongParamsMethod and
params.getEnclosingModule() instanceof ControllerClass
select params,
"By exposing all keys in request parameters or by blindy accessing them, unintended parameters could be used and lead to mass-assignment or have other unexpected side-effects."
class ModelClass extends ModuleBase {
ModelClass() {
this.getModule().getSuperClass+().toString() = "ViewModel" or
this.getModule().getSuperClass+().getAnIncludedModule().toString() = "ActionModel::Model"
}
}
class ModelClassMethodArgument extends DataFlow::Node {
private DataFlow::CallNode call;
ModelClassMethodArgument() {
this = call.getArgument(_) and
call.getExprNode().getNode().getParent+() instanceof ModelClass
}
}
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "Configuration" }
override predicate isSource(DataFlow::Node node) { node.asExpr().getNode() instanceof WeakParams }
// the sink is an instance of a Model class that receives a method call
override predicate isSink(DataFlow::Node node) { node instanceof ModelClassMethodArgument }
}
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
where config.hasFlowPath(source, sink)
select sink.getNode().(ModelClassMethodArgument), source, sink, "This is bad"
// from WeakParams params
// where
// not params.getEnclosingMethod() instanceof StrongParamsMethod and
// params.getEnclosingModule() instanceof ControllerClass
// select params,
// "By exposing all keys in request parameters or by blindy accessing them, unintended parameters could be used and lead to mass-assignment or have other unexpected side-effects."