diff --git a/ql/src/codeql_ruby/ast/Variable.qll b/ql/src/codeql_ruby/ast/Variable.qll index 8b524b504db..7ede082bb4d 100644 --- a/ql/src/codeql_ruby/ast/Variable.qll +++ b/ql/src/codeql_ruby/ast/Variable.qll @@ -28,6 +28,9 @@ class VariableScope extends TScope { result = this.getAVariable() and result.getName() = name } + + /** Gets the scope in which this scope is nested, if any. */ + VariableScope getOuterScope() { result = enclosingScope(this.getScopeElement()) } } /** A variable declared in a scope. */ @@ -85,6 +88,23 @@ class GlobalVariable extends Variable, TGlobalVariable { final override GlobalVariableAccess getAnAccess() { result.getVariable() = this } } +/** An instance variable. */ +class InstanceVariable extends Variable, TInstanceVariable { + override InstanceVariable::Range range; + + /** Holds is this variable is a class instance variable. */ + final predicate isClassInstanceVariable() { range.isClassInstanceVariable() } + + final override InstanceVariableAccess getAnAccess() { result.getVariable() = this } +} + +/** A class variable. */ +class ClassVariable extends Variable, TClassVariable { + override ClassVariable::Range range; + + final override ClassVariableAccess getAnAccess() { result.getVariable() = this } +} + /** An access to a variable. */ class VariableAccess extends Expr { override VariableAccess::Range range; @@ -146,7 +166,6 @@ class VariableReadAccess extends VariableAccess { class LocalVariableAccess extends VariableAccess, @token_identifier { final override LocalVariableAccess::Range range; - /** Gets the variable this identifier refers to. */ final override LocalVariable getVariable() { result = range.getVariable() } final override string getAPrimaryQlClass() { @@ -177,11 +196,10 @@ class LocalVariableWriteAccess extends LocalVariableAccess, VariableWriteAccess /** An access to a local variable where the value is read. */ class LocalVariableReadAccess extends LocalVariableAccess, VariableReadAccess { } -/** An access to a local variable. */ +/** An access to a global variable. */ class GlobalVariableAccess extends VariableAccess, @token_global_variable { final override GlobalVariableAccess::Range range; - /** Gets the variable this identifier refers to. */ final override GlobalVariable getVariable() { result = range.getVariable() } final override string getAPrimaryQlClass() { result = "GlobalVariableAccess" } @@ -192,3 +210,21 @@ class GlobalVariableWriteAccess extends GlobalVariableAccess, VariableWriteAcces /** An access to a global variable where the value is read. */ class GlobalVariableReadAccess extends GlobalVariableAccess, VariableReadAccess { } + +/** An access to an instance variable. */ +class InstanceVariableAccess extends VariableAccess, @token_instance_variable { + final override InstanceVariableAccess::Range range; + + final override InstanceVariable getVariable() { result = range.getVariable() } + + final override string getAPrimaryQlClass() { result = "InstanceVariableAccess" } +} + +/** An access to a class variable. */ +class ClassVariableAccess extends VariableAccess, @token_class_variable { + final override ClassVariableAccess::Range range; + + final override ClassVariable getVariable() { result = range.getVariable() } + + final override string getAPrimaryQlClass() { result = "ClassVariableAccess" } +} diff --git a/ql/src/codeql_ruby/ast/internal/Variable.qll b/ql/src/codeql_ruby/ast/internal/Variable.qll index 85e0f996c55..6128bfa5ef1 100644 --- a/ql/src/codeql_ruby/ast/internal/Variable.qll +++ b/ql/src/codeql_ruby/ast/internal/Variable.qll @@ -9,6 +9,45 @@ private Generated::AstNode parent(Generated::AstNode n) { not n = any(VariableScope s).getScopeElement() } +private predicate instanceVariableAccess( + Generated::InstanceVariable var, string name, VariableScope scope, boolean instance +) { + name = var.getValue() and + scope = enclosingModuleOrClass(var) and + if hasEnclosingMethod(var) then instance = true else instance = false +} + +private predicate classVariableAccess(Generated::ClassVariable var, string name, VariableScope scope) { + name = var.getValue() and + scope = enclosingModuleOrClass(var) +} + +predicate hasEnclosingMethod(Generated::AstNode node) { + exists(Callable method | parentCallableScope*(enclosingScope(node)) = TCallableScope(method) | + method instanceof Method or + method instanceof SingletonMethod + ) +} + +private TCallableScope parentCallableScope(TCallableScope scope) { + exists(Callable c | + scope = TCallableScope(c) and + not c instanceof Method and + not c instanceof SingletonMethod + | + result = scope.(VariableScope).getOuterScope() + ) +} + +private VariableScope parentScope(VariableScope scope) { + not scope instanceof ModuleOrClassScope and + result = scope.getOuterScope() +} + +private ModuleOrClassScope enclosingModuleOrClass(Generated::AstNode node) { + result = parentScope*(enclosingScope(node)) +} + private predicate parameterAssignment( CallableScope::Range scope, string name, Generated::Identifier i ) { @@ -20,7 +59,6 @@ private predicate parameterAssignment( private predicate scopeDefinesParameterVariable( CallableScope::Range scope, string name, Generated::Identifier i ) { - parameterAssignment(scope, name, i) and // In case of overlapping parameter names (e.g. `_`), only the first // parameter will give rise to a variable i = @@ -69,9 +107,6 @@ private class CapturingScope extends VariableScope { ) } - /** Gets the scope in which this scope is nested, if any. */ - private VariableScope getOuterScope() { result = enclosingScope(this.getScopeElement()) } - /** Holds if this scope inherits `name` from an outer scope `outer`. */ predicate inherits(string name, VariableScope outer) { not scopeDefinesParameterVariable(this, name, _) and @@ -112,10 +147,25 @@ private module Cached { cached newtype TVariable = TGlobalVariable(string name) { name = any(Generated::GlobalVariable var).getValue() } or + TClassVariable(VariableScope scope, string name, Generated::AstNode decl) { + decl = + min(Generated::ClassVariable other | + classVariableAccess(other, name, scope) + | + other order by other.getLocation().getStartLine(), other.getLocation().getStartColumn() + ) + } or + TInstanceVariable(VariableScope scope, string name, boolean instance, Generated::AstNode decl) { + decl = + min(Generated::InstanceVariable other | + instanceVariableAccess(other, name, scope, instance) + | + other order by other.getLocation().getStartLine(), other.getLocation().getStartColumn() + ) + } or TLocalVariable(VariableScope scope, string name, Generated::Identifier i) { scopeDefinesParameterVariable(scope, name, i) or - scopeAssigns(scope, name, i) and i = min(Generated::Identifier other | scopeAssigns(scope, name, other) @@ -307,6 +357,22 @@ private module Cached { predicate isCapturedAccess(LocalVariableAccess::Range access) { access.getVariable().getDeclaringScope() != enclosingScope(access) } + + cached + predicate instanceVariableAccess(Generated::InstanceVariable var, InstanceVariable v) { + exists(string name, VariableScope scope, boolean instance | + v = TInstanceVariable(scope, name, instance, _) and + instanceVariableAccess(var, name, scope, instance) + ) + } + + cached + predicate classVariableAccess(Generated::ClassVariable var, ClassVariable variable) { + exists(VariableScope scope, string name | + variable = TClassVariable(scope, name, _) and + classVariableAccess(var, name, scope) + ) + } } import Cached @@ -416,6 +482,43 @@ module GlobalVariable { } } +private class ModuleOrClassScope = TClassScope or TModuleScope or TTopLevelScope; + +module InstanceVariable { + class Range extends Variable::Range, TInstanceVariable { + private ModuleOrClassScope scope; + private boolean instance; + private string name; + private Generated::AstNode decl; + + Range() { this = TInstanceVariable(scope, name, instance, decl) } + + final override string getName() { result = name } + + final predicate isClassInstanceVariable() { instance = false } + + final override Location getLocation() { result = decl.getLocation() } + + final override VariableScope getDeclaringScope() { result = scope } + } +} + +module ClassVariable { + class Range extends Variable::Range, TClassVariable { + private ModuleOrClassScope scope; + private string name; + private Generated::AstNode decl; + + Range() { this = TClassVariable(scope, name, decl) } + + final override string getName() { result = name } + + final override Location getLocation() { result = decl.getLocation() } + + final override VariableScope getDeclaringScope() { result = scope } + } +} + module VariableAccess { abstract class Range extends Expr::Range { abstract Variable getVariable(); @@ -451,3 +554,23 @@ module GlobalVariableAccess { final override GlobalVariable getVariable() { result = variable } } } + +module InstanceVariableAccess { + class Range extends VariableAccess::Range, @token_instance_variable { + InstanceVariable variable; + + Range() { instanceVariableAccess(this, variable) } + + final override InstanceVariable getVariable() { result = variable } + } +} + +module ClassVariableAccess { + class Range extends VariableAccess::Range, @token_class_variable { + ClassVariable variable; + + Range() { classVariableAccess(this, variable) } + + final override ClassVariable getVariable() { result = variable } + } +} diff --git a/ql/test/library-tests/variables/class_variables.rb b/ql/test/library-tests/variables/class_variables.rb new file mode 100644 index 00000000000..dd32df7ea14 --- /dev/null +++ b/ql/test/library-tests/variables/class_variables.rb @@ -0,0 +1,29 @@ +@@x = 42 + +p @@x + +def print + p @@x +end + +class X + def b + p @@x + end + def self.s + p @@x + end +end + +class Y < BasicObject + @@x = 10 +end + +module M + @@x = 12 +end + +module N + include M + p @@x +end diff --git a/ql/test/library-tests/variables/instance_variables.rb b/ql/test/library-tests/variables/instance_variables.rb new file mode 100644 index 00000000000..d35b6b15af9 --- /dev/null +++ b/ql/test/library-tests/variables/instance_variables.rb @@ -0,0 +1,44 @@ +@top = 1 + +def foo + @foo = 10 +end + +def print_foo + puts @foo +end + +puts @top + +class X + @x = 10 + def m() + @y = 7 + end +end + +module M + @m = 10 + def n() + @n = 7 + end +end + +puts { + @x = 100 +} + +def bar + 1.times { @x = 200 } +end + +class C + @x = 42 + def x + def y + @x = 10 + end + y + p @x + end + end \ No newline at end of file diff --git a/ql/test/library-tests/variables/varaccess.expected b/ql/test/library-tests/variables/varaccess.expected index 8791c560a5d..568f4aecffd 100644 --- a/ql/test/library-tests/variables/varaccess.expected +++ b/ql/test/library-tests/variables/varaccess.expected @@ -1,4 +1,25 @@ variableAccess +| class_variables.rb:1:1:1:3 | @@x | class_variables.rb:1:1:1:3 | @@x | class_variables.rb:1:1:29:4 | top-level scope | +| class_variables.rb:3:3:3:5 | @@x | class_variables.rb:1:1:1:3 | @@x | class_variables.rb:1:1:29:4 | top-level scope | +| class_variables.rb:6:4:6:6 | @@x | class_variables.rb:1:1:1:3 | @@x | class_variables.rb:1:1:29:4 | top-level scope | +| class_variables.rb:11:7:11:9 | @@x | class_variables.rb:11:7:11:9 | @@x | class_variables.rb:9:1:16:3 | class scope | +| class_variables.rb:14:6:14:8 | @@x | class_variables.rb:11:7:11:9 | @@x | class_variables.rb:9:1:16:3 | class scope | +| class_variables.rb:19:3:19:5 | @@x | class_variables.rb:19:3:19:5 | @@x | class_variables.rb:18:1:20:3 | class scope | +| class_variables.rb:23:3:23:5 | @@x | class_variables.rb:23:3:23:5 | @@x | class_variables.rb:22:1:24:3 | module scope | +| class_variables.rb:28:5:28:7 | @@x | class_variables.rb:28:5:28:7 | @@x | class_variables.rb:26:1:29:3 | module scope | +| instance_variables.rb:1:1:1:4 | @top | instance_variables.rb:1:1:1:4 | @top | instance_variables.rb:1:1:44:4 | top-level scope | +| instance_variables.rb:4:3:4:6 | @foo | instance_variables.rb:4:3:4:6 | @foo | instance_variables.rb:1:1:44:4 | top-level scope | +| instance_variables.rb:8:8:8:11 | @foo | instance_variables.rb:4:3:4:6 | @foo | instance_variables.rb:1:1:44:4 | top-level scope | +| instance_variables.rb:11:6:11:9 | @top | instance_variables.rb:1:1:1:4 | @top | instance_variables.rb:1:1:44:4 | top-level scope | +| instance_variables.rb:14:3:14:4 | @x | instance_variables.rb:14:3:14:4 | @x | instance_variables.rb:13:1:18:3 | class scope | +| instance_variables.rb:16:5:16:6 | @y | instance_variables.rb:16:5:16:6 | @y | instance_variables.rb:13:1:18:3 | class scope | +| instance_variables.rb:21:2:21:3 | @m | instance_variables.rb:21:2:21:3 | @m | instance_variables.rb:20:1:25:3 | module scope | +| instance_variables.rb:23:4:23:5 | @n | instance_variables.rb:23:4:23:5 | @n | instance_variables.rb:20:1:25:3 | module scope | +| instance_variables.rb:28:3:28:4 | @x | instance_variables.rb:28:3:28:4 | @x | instance_variables.rb:1:1:44:4 | top-level scope | +| instance_variables.rb:32:12:32:13 | @x | instance_variables.rb:32:12:32:13 | @x | instance_variables.rb:1:1:44:4 | top-level scope | +| instance_variables.rb:36:3:36:4 | @x | instance_variables.rb:36:3:36:4 | @x | instance_variables.rb:35:1:44:4 | class scope | +| instance_variables.rb:39:6:39:7 | @x | instance_variables.rb:39:6:39:7 | @x | instance_variables.rb:35:1:44:4 | class scope | +| instance_variables.rb:42:6:42:7 | @x | instance_variables.rb:39:6:39:7 | @x | instance_variables.rb:35:1:44:4 | class scope | | nested_scopes.rb:5:3:5:3 | a | nested_scopes.rb:5:3:5:3 | a | nested_scopes.rb:4:1:39:3 | class scope | | nested_scopes.rb:7:5:7:5 | a | nested_scopes.rb:7:5:7:5 | a | nested_scopes.rb:6:3:37:5 | module scope | | nested_scopes.rb:9:7:9:7 | a | nested_scopes.rb:9:7:9:7 | a | nested_scopes.rb:8:5:35:7 | module scope | @@ -221,6 +242,27 @@ implicitWrite | ssa.rb:64:8:64:8 | a | | ssa.rb:66:15:66:15 | a | readAccess +| class_variables.rb:1:1:1:3 | @@x | +| class_variables.rb:3:3:3:5 | @@x | +| class_variables.rb:6:4:6:6 | @@x | +| class_variables.rb:11:7:11:9 | @@x | +| class_variables.rb:14:6:14:8 | @@x | +| class_variables.rb:19:3:19:5 | @@x | +| class_variables.rb:23:3:23:5 | @@x | +| class_variables.rb:28:5:28:7 | @@x | +| instance_variables.rb:1:1:1:4 | @top | +| instance_variables.rb:4:3:4:6 | @foo | +| instance_variables.rb:8:8:8:11 | @foo | +| instance_variables.rb:11:6:11:9 | @top | +| instance_variables.rb:14:3:14:4 | @x | +| instance_variables.rb:16:5:16:6 | @y | +| instance_variables.rb:21:2:21:3 | @m | +| instance_variables.rb:23:4:23:5 | @n | +| instance_variables.rb:28:3:28:4 | @x | +| instance_variables.rb:32:12:32:13 | @x | +| instance_variables.rb:36:3:36:4 | @x | +| instance_variables.rb:39:6:39:7 | @x | +| instance_variables.rb:42:6:42:7 | @x | | nested_scopes.rb:14:16:14:16 | a | | nested_scopes.rb:15:11:15:11 | a | | nested_scopes.rb:16:13:16:13 | a | diff --git a/ql/test/library-tests/variables/variable.expected b/ql/test/library-tests/variables/variable.expected index 54600ae9da1..239e60aacef 100644 --- a/ql/test/library-tests/variables/variable.expected +++ b/ql/test/library-tests/variables/variable.expected @@ -1,5 +1,20 @@ +| class_variables.rb:1:1:1:3 | @@x | +| class_variables.rb:11:7:11:9 | @@x | +| class_variables.rb:19:3:19:5 | @@x | +| class_variables.rb:23:3:23:5 | @@x | +| class_variables.rb:28:5:28:7 | @@x | | file://:0:0:0:0 | $0 | | file://:0:0:0:0 | $global | +| instance_variables.rb:1:1:1:4 | @top | +| instance_variables.rb:4:3:4:6 | @foo | +| instance_variables.rb:14:3:14:4 | @x | +| instance_variables.rb:16:5:16:6 | @y | +| instance_variables.rb:21:2:21:3 | @m | +| instance_variables.rb:23:4:23:5 | @n | +| instance_variables.rb:28:3:28:4 | @x | +| instance_variables.rb:32:12:32:13 | @x | +| instance_variables.rb:36:3:36:4 | @x | +| instance_variables.rb:39:6:39:7 | @x | | nested_scopes.rb:5:3:5:3 | a | | nested_scopes.rb:7:5:7:5 | a | | nested_scopes.rb:9:7:9:7 | a | diff --git a/ql/test/library-tests/variables/varscopes.expected b/ql/test/library-tests/variables/varscopes.expected index 104f0ded8b6..1cc0d4913e9 100644 --- a/ql/test/library-tests/variables/varscopes.expected +++ b/ql/test/library-tests/variables/varscopes.expected @@ -1,4 +1,25 @@ +| class_variables.rb:1:1:29:4 | top-level scope | +| class_variables.rb:5:1:7:3 | method scope | +| class_variables.rb:9:1:16:3 | class scope | +| class_variables.rb:10:3:12:5 | method scope | +| class_variables.rb:13:3:15:5 | method scope | +| class_variables.rb:18:1:20:3 | class scope | +| class_variables.rb:22:1:24:3 | module scope | +| class_variables.rb:26:1:29:3 | module scope | | file://:0:0:0:0 | global scope | +| instance_variables.rb:1:1:44:4 | top-level scope | +| instance_variables.rb:3:1:5:3 | method scope | +| instance_variables.rb:7:1:9:3 | method scope | +| instance_variables.rb:13:1:18:3 | class scope | +| instance_variables.rb:15:3:17:5 | method scope | +| instance_variables.rb:20:1:25:3 | module scope | +| instance_variables.rb:22:2:24:4 | method scope | +| instance_variables.rb:27:6:29:1 | block scope | +| instance_variables.rb:31:1:33:3 | method scope | +| instance_variables.rb:32:10:32:21 | block scope | +| instance_variables.rb:35:1:44:4 | class scope | +| instance_variables.rb:37:3:43:5 | method scope | +| instance_variables.rb:38:4:40:6 | method scope | | nested_scopes.rb:1:1:3:3 | method scope | | nested_scopes.rb:1:1:42:1 | top-level scope | | nested_scopes.rb:4:1:39:3 | class scope |