Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ module ControlFlow {
* Only nodes that can be reached from the callable entry point are included in
* the CFG.
*/
class Node extends TNode {
class Node extends TCfgNode {
/** Gets a textual representation of this control flow node. */
string toString() { none() }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ private module Cached {
* The control flow graph is pruned for unreachable nodes.
*/
cached
newtype TNode =
newtype TCfgNode =
TEntryNode(CfgScope scope) { succEntrySplits(scope, _, _, _) } or
TAnnotatedExitNode(CfgScope scope, boolean normal) {
exists(Reachability::SameSplitsBlock b, SuccessorType t | b.isReachable(_) |
Expand All @@ -807,7 +807,7 @@ private module Cached {

/** Gets a successor node of a given flow type, if any. */
cached
TNode getASuccessor(TNode pred, SuccessorType t) {
TCfgNode getASuccessor(TCfgNode pred, SuccessorType t) {
// Callable entry node -> callable body
exists(ControlFlowElement succElement, Splits succSplits, CfgScope scope |
result = TElementNode(succElement, succSplits) and
Expand Down
35 changes: 23 additions & 12 deletions ruby/ql/lib/codeql/ruby/AST.qll
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,27 @@ private import ast.internal.Scope
private import ast.internal.Synthesis
private import ast.internal.TreeSitter

cached
private module Cached {
cached
ModuleBase getEnclosingModule(Scope s) {
result = s
or
not s instanceof ModuleBase and result = getEnclosingModule(s.getOuterScope())
}

cached
MethodBase getEnclosingMethod(Scope s) {
result = s
or
not s instanceof MethodBase and
not s instanceof ModuleBase and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this line needed? (Maybe add a comment?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it is not needed, since a module cannot be inside a method (right?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like singleton classes can be inside methods. Here's one example I found: https://github.com/gitlabhq/gitlabhq/blob/94ecc00f47df7051eea905a5899053bf476e0589/app/services/users/signup_service.rb#L27-L32

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A module can be created in the body of a method, however, from a scope point of view the method and the module are unrelated.

result = getEnclosingMethod(s.getOuterScope())
}
}

private import Cached

/**
* A node in the abstract syntax tree. This class is the base class for all Ruby
* program elements.
Expand All @@ -39,20 +60,10 @@ class AstNode extends TAstNode {
final string getPrimaryQlClasses() { result = concat(this.getAPrimaryQlClass(), ",") }

/** Gets the enclosing module, if any. */
ModuleBase getEnclosingModule() {
exists(Scope::Range s |
s = scopeOf(toGeneratedInclSynth(this)) and
toGeneratedInclSynth(result) = s.getEnclosingModule()
)
}
ModuleBase getEnclosingModule() { result = getEnclosingModule(scopeOfInclSynth(this)) }

/** Gets the enclosing method, if any. */
MethodBase getEnclosingMethod() {
exists(Scope::Range s |
s = scopeOf(toGeneratedInclSynth(this)) and
toGeneratedInclSynth(result) = s.getEnclosingMethod()
)
}
MethodBase getEnclosingMethod() { result = getEnclosingMethod(scopeOfInclSynth(this)) }

/** Gets a textual representation of this node. */
cached
Expand Down
2 changes: 1 addition & 1 deletion ruby/ql/lib/codeql/ruby/ast/Control.qll
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ class ForExpr extends Loop, TForExpr {
final override string getAPrimaryQlClass() { result = "ForExpr" }

/** Gets the body of this `for` loop. */
final override Stmt getBody() { toGenerated(result) = g.getBody() }
final override StmtSequence getBody() { toGenerated(result) = g.getBody() }

/** Gets the pattern representing the iteration argument. */
final Pattern getPattern() { toGenerated(result) = g.getPattern() }
Expand Down
98 changes: 18 additions & 80 deletions ruby/ql/lib/codeql/ruby/ast/Expr.qll
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
private import codeql.ruby.AST
private import codeql.ruby.CFG
private import internal.AST
private import internal.Expr
private import internal.TreeSitter

/**
Expand Down Expand Up @@ -91,90 +92,19 @@ class StmtSequence extends Expr, TStmtSequence {
}
}

private class StmtSequenceSynth extends StmtSequence, TStmtSequenceSynth {
final override Stmt getStmt(int n) { synthChild(this, n, result) }

final override string toString() { result = "..." }
}

private class Then extends StmtSequence, TThen {
private Ruby::Then g;

Then() { this = TThen(g) }

override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }

final override string toString() { result = "then ..." }
}

private class Else extends StmtSequence, TElse {
private Ruby::Else g;

Else() { this = TElse(g) }

override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }

final override string toString() { result = "else ..." }
}

private class Do extends StmtSequence, TDo {
private Ruby::Do g;

Do() { this = TDo(g) }

override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }

final override string toString() { result = "do ..." }
}

private class Ensure extends StmtSequence, TEnsure {
private Ruby::Ensure g;

Ensure() { this = TEnsure(g) }

override Stmt getStmt(int n) { toGenerated(result) = g.getChild(n) }

final override string toString() { result = "ensure ..." }
}

/**
* A sequence of statements representing the body of a method, class, module,
* or do-block. That is, any body that may also include rescue/ensure/else
* statements.
*/
class BodyStmt extends StmtSequence, TBodyStmt {
// Not defined by dispatch, as it should not be exposed
private Ruby::AstNode getChild(int i) {
result = any(Ruby::Method g | this = TMethod(g)).getChild(i)
or
result = any(Ruby::SingletonMethod g | this = TSingletonMethod(g)).getChild(i)
or
exists(Ruby::Lambda g | this = TLambda(g) |
result = g.getBody().(Ruby::DoBlock).getChild(i) or
result = g.getBody().(Ruby::Block).getChild(i)
)
or
result = any(Ruby::DoBlock g | this = TDoBlock(g)).getChild(i)
or
result = any(Ruby::Program g | this = TToplevel(g)).getChild(i) and
not result instanceof Ruby::BeginBlock
or
result = any(Ruby::Class g | this = TClassDeclaration(g)).getChild(i)
or
result = any(Ruby::SingletonClass g | this = TSingletonClass(g)).getChild(i)
or
result = any(Ruby::Module g | this = TModuleDeclaration(g)).getChild(i)
or
result = any(Ruby::Begin g | this = TBeginExpr(g)).getChild(i)
}

final override Stmt getStmt(int n) {
result =
rank[n + 1](AstNode node, int i |
toGenerated(node) = this.getChild(i) and
not node instanceof Else and
not node instanceof RescueClause and
not node instanceof Ensure
toGenerated(result) =
rank[n + 1](Ruby::AstNode node, int i |
node = getBodyStmtChild(this, i) and
not node instanceof Ruby::Else and
not node instanceof Ruby::Rescue and
not node instanceof Ruby::Ensure
|
node order by i
)
Expand All @@ -183,17 +113,25 @@ class BodyStmt extends StmtSequence, TBodyStmt {
/** Gets the `n`th rescue clause in this block. */
final RescueClause getRescue(int n) {
result =
rank[n + 1](RescueClause node, int i | toGenerated(node) = this.getChild(i) | node order by i)
rank[n + 1](RescueClause node, int i |
toGenerated(node) = getBodyStmtChild(this, i)
|
node order by i
)
}

/** Gets a rescue clause in this block. */
final RescueClause getARescue() { result = this.getRescue(_) }

/** Gets the `else` clause in this block, if any. */
final StmtSequence getElse() { result = unique(Else s | toGenerated(s) = getChild(_)) }
final StmtSequence getElse() {
result = unique(Else s | toGenerated(s) = getBodyStmtChild(this, _))
}

/** Gets the `ensure` clause in this block, if any. */
final StmtSequence getEnsure() { result = unique(Ensure s | toGenerated(s) = getChild(_)) }
final StmtSequence getEnsure() {
result = unique(Ensure s | toGenerated(s) = getBodyStmtChild(this, _))
}

final predicate hasEnsure() { exists(this.getEnsure()) }

Expand Down
11 changes: 1 addition & 10 deletions ruby/ql/lib/codeql/ruby/ast/Method.qll
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ private import codeql.ruby.AST
private import codeql.ruby.controlflow.ControlFlowGraph
private import internal.AST
private import internal.TreeSitter
private import internal.Method

/** A callable. */
class Callable extends StmtSequence, Expr, Scope, TCallable {
Expand Down Expand Up @@ -212,16 +213,6 @@ class DoBlock extends Block, BodyStmt, TDoBlock {
* ```
*/
class BraceBlock extends Block, TBraceBlock {
private Ruby::Block g;

BraceBlock() { this = TBraceBlock(g) }

final override Parameter getParameter(int n) {
toGenerated(result) = g.getParameters().getChild(n)
}

final override Stmt getStmt(int i) { toGenerated(result) = g.getChild(i) }

final override string toString() { result = "{ ... }" }

final override string getAPrimaryQlClass() { result = "BraceBlock" }
Expand Down
18 changes: 10 additions & 8 deletions ruby/ql/lib/codeql/ruby/ast/Parameter.qll
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ class NamedParameter extends Parameter, TNamedParameter {
final VariableAccess getAnAccess() { result = this.getVariable().getAnAccess() }

/** Gets the access that defines the underlying local variable. */
final VariableAccess getDefiningAccess() { result = this.getVariable().getDefiningAccess() }
final VariableAccess getDefiningAccess() {
result = this.getVariable().getDefiningAccess()
or
result = this.(SimpleParameterSynthImpl).getDefininingAccess()
}

override AstNode getAChild(string pred) {
result = super.getAChild(pred)
Expand All @@ -68,14 +72,12 @@ class NamedParameter extends Parameter, TNamedParameter {
}

/** A simple (normal) parameter. */
class SimpleParameter extends NamedParameter, PatternParameter, VariablePattern, TSimpleParameter {
private Ruby::Identifier g;

SimpleParameter() { this = TSimpleParameter(g) }
class SimpleParameter extends NamedParameter, PatternParameter, VariablePattern, TSimpleParameter instanceof SimpleParameterImpl {
final override string getName() { result = SimpleParameterImpl.super.getNameImpl() }

final override string getName() { result = g.getValue() }

final override LocalVariable getVariable() { result = TLocalVariableReal(_, _, g) }
final override LocalVariable getVariable() {
result = SimpleParameterImpl.super.getVariableImpl()
}

final override LocalVariable getAVariable() { result = this.getVariable() }

Expand Down
3 changes: 3 additions & 0 deletions ruby/ql/lib/codeql/ruby/ast/Pattern.qll
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ private import internal.AST
private import internal.Pattern
private import internal.TreeSitter
private import internal.Variable
private import internal.Parameter

/** A pattern. */
class Pattern extends AstNode {
Expand All @@ -15,6 +16,8 @@ class Pattern extends AstNode {
implicitParameterAssignmentNode(toGenerated(this), _)
or
this = getSynthChild(any(AssignExpr ae), 0)
or
this instanceof SimpleParameterImpl
}

/** Gets a variable used in (or introduced by) this pattern. */
Expand Down
26 changes: 12 additions & 14 deletions ruby/ql/lib/codeql/ruby/ast/Scope.qll
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,20 @@ private import internal.AST
private import internal.Scope
private import internal.TreeSitter

class Scope extends AstNode, TScopeType {
private Scope::Range range;
/**
* A variable scope. This is either a top-level (file), a module, a class,
* or a callable.
*/
class Scope extends AstNode, TScopeType instanceof ScopeImpl {
/** Gets the outer scope, if any. */
Scope getOuterScope() { result = super.getOuterScopeImpl() }

Scope() { range = toGenerated(this) }
/** Gets a variable declared in this scope. */
Variable getAVariable() { result = super.getAVariableImpl() }

/** Gets the scope in which this scope is nested, if any. */
Scope getOuterScope() { toGenerated(result) = range.getOuterScope() }

/** Gets a variable that is declared in this scope. */
final Variable getAVariable() { result.getDeclaringScope() = this }

/** Gets the variable declared in this scope with the given name, if any. */
final Variable getVariable(string name) {
result = this.getAVariable() and
result.getName() = name
}
/** Gets the variable named `name` declared in this scope. */
Variable getVariable(string name) { result = super.getVariableImpl(name) }
}

/** A scope in which a `self` variable exists. */
class SelfScope extends Scope, TSelfScopeType { }
19 changes: 9 additions & 10 deletions ruby/ql/lib/codeql/ruby/ast/Variable.qll
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ private import codeql.Locations
private import internal.AST
private import internal.TreeSitter
private import internal.Variable
private import internal.Parameter

/** A variable declared in a scope. */
class Variable instanceof VariableImpl {
Expand Down Expand Up @@ -50,7 +51,7 @@ class LocalVariable extends Variable, TLocalVariable {
*
* `x` is a captured variable, whereas `y` is not.
*/
predicate isCaptured() { this.getAnAccess().isCapturedAccess() }
final predicate isCaptured() { this.getAnAccess().isCapturedAccess() }
}

/** A global variable. */
Expand Down Expand Up @@ -110,7 +111,11 @@ class VariableAccess extends Expr instanceof VariableAccessImpl {
* the access to `elements` in the parameter list is an implicit assignment,
* as is the first access to `e`.
*/
predicate isImplicitWrite() { implicitWriteAccess(toGenerated(this)) }
predicate isImplicitWrite() {
implicitWriteAccess(toGenerated(this))
or
this = any(SimpleParameterSynthImpl p).getDefininingAccess()
}

final override string toString() { result = VariableAccessImpl.super.toString() }
}
Expand Down Expand Up @@ -147,7 +152,7 @@ class LocalVariableAccess extends VariableAccess instanceof LocalVariableAccessI
* the access to `x` in the first `puts x` is a captured access, while
* the access to `x` in the second `puts x` is not.
*/
predicate isCapturedAccess() { isCapturedAccess(this) }
final predicate isCapturedAccess() { isCapturedAccess(this) }
}

/** An access to a local variable where the value is updated. */
Expand Down Expand Up @@ -195,10 +200,4 @@ class SelfVariableAccess extends LocalVariableAccess instanceof SelfVariableAcce
}

/** An access to the `self` variable where the value is read. */
class SelfVariableReadAccess extends SelfVariableAccess, VariableReadAccess {
// We override the definition in `LocalVariableAccess` because it gives the
// wrong result for synthesised `self` variables.
override predicate isCapturedAccess() {
this.getVariable().getDeclaringScope() != this.getCfgScope()
}
}
class SelfVariableReadAccess extends SelfVariableAccess, VariableReadAccess { }
Loading