Skip to content

Commit 0613fda

Browse files
committed
Ruby: separate constant propagation of regexps from strings
1 parent e12b6df commit 0613fda

26 files changed

+169
-78
lines changed

ruby/ql/lib/codeql/ruby/Concepts.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ module HTTP {
268268
string getUrlPattern() {
269269
exists(CfgNodes::ExprNodes::StringlikeLiteralCfgNode strNode |
270270
this.getUrlPatternArg().getALocalSource() = DataFlow::exprNode(strNode) and
271-
result = strNode.getExpr().getConstantValue().getStringOrSymbol()
271+
result = strNode.getExpr().getConstantValue().getStringlikeValue()
272272
)
273273
}
274274

@@ -431,7 +431,7 @@ module HTTP {
431431
string getMimetype() {
432432
exists(CfgNodes::ExprNodes::StringlikeLiteralCfgNode strNode |
433433
this.getMimetypeOrContentTypeArg().getALocalSource() = DataFlow::exprNode(strNode) and
434-
result = strNode.getExpr().getConstantValue().getStringOrSymbol().splitAt(";", 0)
434+
result = strNode.getExpr().getConstantValue().getStringlikeValue().splitAt(";", 0)
435435
)
436436
or
437437
not exists(this.getMimetypeOrContentTypeArg()) and

ruby/ql/lib/codeql/ruby/ast/Constant.qll

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ class ConstantValue extends TConstantValue {
2727
or
2828
result = ":" + this.getSymbol()
2929
or
30+
result = this.getRegExp()
31+
or
3032
result = this.getBoolean().toString()
3133
or
3234
this.isNil() and result = "nil"
@@ -62,11 +64,31 @@ class ConstantValue extends TConstantValue {
6264
/** Holds if this is the symbol value `:s`. */
6365
predicate isSymbol(string s) { s = this.getSymbol() }
6466

65-
/** Gets the string or symbol value, if any. */
66-
string getStringOrSymbol() { result = [this.getString(), this.getSymbol()] }
67+
/** Gets the regexp value, if this is a regexp. */
68+
string getRegExp() { this.isRegExpWithFlags(result, _) }
69+
70+
/** Holds if this is the regexp value `/s/`. */
71+
predicate isRegExp(string s) { this.isRegExpWithFlags(s, _) }
72+
73+
/** Holds if this is the regexp value `/s/flags` . */
74+
predicate isRegExpWithFlags(string s, string flags) { this = TRegExp(s, flags) }
75+
76+
/** DEPRECATED: Use `getStringlikeValue` instead. */
77+
deprecated string getStringOrSymbol() { result = this.getStringlikeValue() }
78+
79+
/** DEPRECATED: Use `isStringlikeValue` instead. */
80+
deprecated predicate isStringOrSymbol(string s) { s = this.getStringlikeValue() }
6781

68-
/** Holds if this is the string value `s` or the symbol value `:s`. */
69-
predicate isStringOrSymbol(string s) { s = this.getStringOrSymbol() }
82+
/** Gets the string/symbol/regexp value, if any. */
83+
string getStringlikeValue() { result = [this.getString(), this.getSymbol(), this.getRegExp()] }
84+
85+
/**
86+
* Holds if this is:
87+
* - the string value `s`,
88+
* - the symbol value `:s`, or
89+
* - the regexp value `/s/`.
90+
*/
91+
predicate isStringlikeValue(string s) { s = this.getStringlikeValue() }
7092

7193
/** Gets the Boolean value, if this is a Boolean. */
7294
boolean getBoolean() { this = TBoolean(result) }
@@ -92,12 +114,18 @@ module ConstantValue {
92114
/** A constant complex value. */
93115
class ConstantComplexValue extends ConstantValue, TComplex { }
94116

117+
/** A constant string-like value. */
118+
class ConstantStringlikeValue extends ConstantValue, TStringlike { }
119+
95120
/** A constant string value. */
96121
class ConstantStringValue extends ConstantValue, TString { }
97122

98123
/** A constant symbol value. */
99124
class ConstantSymbolValue extends ConstantValue, TSymbol { }
100125

126+
/** A constant regexp value. */
127+
class ConstantRegExpValue extends ConstantValue, TRegExp { }
128+
101129
/** A constant Boolean value. */
102130
class ConstantBooleanValue extends ConstantValue, TBoolean { }
103131

ruby/ql/lib/codeql/ruby/ast/Expr.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,11 +454,11 @@ class StringConcatenation extends Expr, TStringConcatenation {
454454
*/
455455
final string getConcatenatedValueText() {
456456
forall(StringLiteral c | c = this.getString(_) |
457-
exists(c.getConstantValue().getStringOrSymbol())
457+
exists(c.getConstantValue().getStringlikeValue())
458458
) and
459459
result =
460460
concat(string valueText, int i |
461-
valueText = this.getString(i).getConstantValue().getStringOrSymbol()
461+
valueText = this.getString(i).getConstantValue().getStringlikeValue()
462462
|
463463
valueText order by i
464464
)

ruby/ql/lib/codeql/ruby/ast/Method.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ private class MethodModifier extends MethodCall {
5353
predicate modifiesMethod(Namespace n, string name) {
5454
this = n.getAStmt() and
5555
[
56-
this.getMethodArgument().getConstantValue().getStringOrSymbol(),
56+
this.getMethodArgument().getConstantValue().getStringlikeValue(),
5757
this.getMethodArgument().(MethodBase).getName()
5858
] = name
5959
}

ruby/ql/lib/codeql/ruby/ast/Pattern.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ class HashPattern extends CasePattern, THashPattern {
278278
/** Gets the value for a given key name. */
279279
CasePattern getValueByKey(string key) {
280280
exists(int i |
281-
this.getKey(i).getConstantValue().isStringOrSymbol(key) and result = this.getValue(i)
281+
this.getKey(i).getConstantValue().isStringlikeValue(key) and result = this.getValue(i)
282282
)
283283
}
284284

ruby/ql/lib/codeql/ruby/ast/internal/Constant.qll

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,16 @@ private module Propagation {
229229
}
230230

231231
pragma[nomagic]
232-
string getNonSymbolValue() {
232+
string getStringValue() {
233233
result = this.getValue() and
234-
not this.getExpr() instanceof SymbolLiteral
234+
not this.getExpr() instanceof SymbolLiteral and
235+
not this.getExpr() instanceof RegExpLiteral
236+
}
237+
238+
pragma[nomagic]
239+
string getRegExpValue(string flags) {
240+
result = this.getValue() and
241+
flags = this.getExpr().(RegExpLiteral).getFlagString()
235242
}
236243
}
237244

@@ -251,7 +258,7 @@ private module Propagation {
251258
s = left + right
252259
)
253260
or
254-
s = e.(StringlikeLiteralWithInterpolationCfgNode).getNonSymbolValue()
261+
s = e.(StringlikeLiteralWithInterpolationCfgNode).getStringValue()
255262
or
256263
// If last statement in the interpolation is a constant or local variable read,
257264
// we attempt to look up its string value.
@@ -267,13 +274,15 @@ private module Propagation {
267274
exists(ExprCfgNode last | last = e.(RegExpInterpolationComponentCfgNode).getLastStmt() |
268275
isInt(last, any(int i | s = i.toString())) or
269276
isFloat(last, any(float f | s = f.toString())) or
270-
isString(last, s)
277+
isString(last, s) or
278+
isRegExp(last, s, _) // Note: we lose the flags for interpolated regexps here.
271279
)
272280
}
273281

274282
private predicate isStringExprNoCfg(Expr e, string s) {
275283
s = e.(StringlikeLiteralImpl).getStringValue() and
276-
not e instanceof SymbolLiteral
284+
not e instanceof SymbolLiteral and
285+
not e instanceof RegExpLiteral
277286
or
278287
s = e.(EncodingLiteralImpl).getValue()
279288
or
@@ -319,6 +328,31 @@ private module Propagation {
319328
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isSymbol(n, s))
320329
}
321330

331+
predicate isRegExp(ExprCfgNode e, string s, string flags) {
332+
isRegExpExprNoCfg(e.getExpr(), s, flags)
333+
or
334+
isRegExpExpr(e.getExpr().(ConstantReadAccess).getValue(), s, flags)
335+
or
336+
isRegExp(getSource(e), s, flags)
337+
or
338+
s = e.(StringlikeLiteralWithInterpolationCfgNode).getRegExpValue(flags)
339+
}
340+
341+
private predicate isRegExpExprNoCfg(Expr e, string s, string flags) {
342+
s = e.(StringlikeLiteralImpl).getStringValue() and
343+
e.(RegExpLiteral).getFlagString() = flags
344+
or
345+
isRegExpExprNoCfg(e.(ConstantReadAccess).getValue(), s, flags)
346+
}
347+
348+
predicate isRegExpExpr(Expr e, string s, string flags) {
349+
isRegExpExprNoCfg(e, s, flags)
350+
or
351+
isRegExpExpr(e.(ConstantReadAccess).getValue(), s, flags)
352+
or
353+
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isRegExp(n, s, flags))
354+
}
355+
322356
predicate isBoolean(ExprCfgNode e, boolean b) {
323357
isBooleanExprNoCfg(e.getExpr(), b)
324358
or
@@ -388,9 +422,18 @@ private module Cached {
388422
s = any(StringComponentImpl c).getValue()
389423
} or
390424
TSymbol(string s) { isString(_, s) or isSymbolExpr(_, s) } or
425+
TRegExp(string s, string flags) {
426+
isRegExp(_, s, flags)
427+
or
428+
isRegExpExpr(_, s, flags)
429+
or
430+
s = any(StringComponentImpl c).getValue() and flags = ""
431+
} or
391432
TBoolean(boolean b) { b in [false, true] } or
392433
TNil()
393434

435+
class TStringlike = TString or TSymbol or TRegExp;
436+
394437
cached
395438
ConstantValue getConstantValue(ExprCfgNode n) {
396439
result.isInt(any(int i | isInt(n, i)))
@@ -411,6 +454,8 @@ private module Cached {
411454
or
412455
result.isSymbol(any(string s | isSymbol(n, s)))
413456
or
457+
exists(string s, string flags | isRegExp(n, s, flags) and result = TRegExp(s, flags))
458+
or
414459
result.isBoolean(any(boolean b | isBoolean(n, b)))
415460
or
416461
result.isNil() and
@@ -437,6 +482,8 @@ private module Cached {
437482
or
438483
result.isSymbol(any(string s | isSymbolExpr(e, s)))
439484
or
485+
exists(string s, string flags | isRegExpExpr(e, s, flags) and result = TRegExp(s, flags))
486+
or
440487
result.isBoolean(any(boolean b | isBooleanExpr(e, b)))
441488
or
442489
result.isNil() and

ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ class RedirectToCall extends ActionControllerContextCall {
189189
/** Gets the `ActionControllerActionMethod` to redirect to, if any */
190190
ActionControllerActionMethod getRedirectActionMethod() {
191191
exists(string methodName |
192-
this.getKeywordArgument("action").getConstantValue().isStringOrSymbol(methodName) and
192+
this.getKeywordArgument("action").getConstantValue().isStringlikeValue(methodName) and
193193
methodName = result.getName() and
194194
result.getEnclosingModule() = this.getControllerClass()
195195
)
@@ -225,7 +225,7 @@ pragma[nomagic]
225225
private predicate actionControllerHasHelperMethodCall(ActionControllerControllerClass c, string name) {
226226
exists(MethodCall mc |
227227
mc.getMethodName() = "helper_method" and
228-
mc.getAnArgument().getConstantValue().isStringOrSymbol(name) and
228+
mc.getAnArgument().getConstantValue().isStringlikeValue(name) and
229229
mc.getEnclosingModule() = c
230230
)
231231
}
@@ -317,7 +317,7 @@ class ActionControllerSkipForgeryProtectionCall extends CSRFProtectionSetting::R
317317
call.getMethodName() = "skip_forgery_protection"
318318
or
319319
call.getMethodName() = "skip_before_action" and
320-
call.getAnArgument().getConstantValue().isStringOrSymbol("verify_authenticity_token")
320+
call.getAnArgument().getConstantValue().isStringlikeValue("verify_authenticity_token")
321321
)
322322
}
323323

0 commit comments

Comments
 (0)