Skip to content

Ruby: Use Type Tracking for ORM flow #15846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hmac wants to merge 1 commit into from
Closed

Ruby: Use Type Tracking for ORM flow #15846

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 21 additions & 26 deletions ruby/ql/lib/codeql/ruby/security/XSS.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ private import codeql.ruby.frameworks.Rails
private import codeql.ruby.dataflow.RemoteFlowSources
private import codeql.ruby.dataflow.BarrierGuards
private import codeql.ruby.dataflow.internal.DataFlowDispatch
private import codeql.ruby.typetracking.TypeTracking

/**
* Provides default sources, sinks and sanitizers for detecting
Expand Down Expand Up @@ -278,30 +279,24 @@ module ReflectedXss {
}
}

private module OrmTracking {
/**
* A data flow configuration to track flow from finder calls to field accesses.
*/
private module Config implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
// We currently only use ORM instances that come from a call site, so restrict the sources
// to calls. This works around a performance issue that would arise from using 'self' as a source
// in ActiveRecord models. Over time, library models should stop relying on OrmInstantiation and instead
// use API graphs or type-tracking the same way we track other types.
source instanceof OrmInstantiation and source instanceof DataFlow::CallNode
}

// Select any call receiver and narrow down later
predicate isSink(DataFlow::Node sink) { sink = any(DataFlow::CallNode c).getReceiver() }

predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
Shared::isAdditionalXssFlowStep(node1, node2)
}

predicate isBarrierIn(DataFlow::Node node) { node instanceof DataFlow::SelfParameterNode }
}
private DataFlow::LocalSourceNode ormInstance(TypeTracker t, OrmInstantiation instance) {
t.start() and
result = instance
or
exists(TypeTracker t2, DataFlow::LocalSourceNode mid |
t = t2.step(ormInstance(t2, instance), mid) and
(
result = mid
or
Shared::isAdditionalXssFlowStep(mid, result)
)
)
}

import DataFlow::Global<Config>
private DataFlow::Node ormInstance(OrmInstantiation i) {
exists(DataFlow::Node mid | ormInstance(TypeTracker::end(), i).flowsTo(mid) |
result = mid or Shared::isAdditionalXssFlowStep(mid, result)
)
}

/** Provides default sources, sinks and sanitizers for detecting stored cross-site scripting (XSS) vulnerabilities. */
Expand All @@ -322,9 +317,9 @@ module StoredXss {

private class OrmFieldAsSource extends Source instanceof DataFlow::CallNode {
OrmFieldAsSource() {
exists(DataFlow::CallNode subSrc |
OrmTracking::flow(subSrc, this.getReceiver()) and
subSrc.(OrmInstantiation).methodCallMayAccessField(this.getMethodName()) and
exists(OrmInstantiation instance |
this.getReceiver() = ormInstance(instance) and
instance.methodCallMayAccessField(this.getMethodName()) and
this.getNumberOfArguments() = 0 and
not exists(this.getBlock())
)
Expand Down