Skip to content

Commit 0bbdc70

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#3864 from erik-krogh/exprString
Approved by asgerf, esbena
2 parents dd1a8e9 + ceb1929 commit 0bbdc70

File tree

5 files changed

+146
-20
lines changed

5 files changed

+146
-20
lines changed

‎javascript/ql/src/semmle/javascript/Expr.qll

Lines changed: 63 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ class Expr extends @expr, ExprOrStmt, ExprOrType, AST::ValueNode {
109109

110110
/** Gets the constant string value this expression evaluates to, if any. */
111111
cached
112-
string getStringValue() { none() }
112+
string getStringValue() { result = getStringValue(this) }
113113

114114
/** Holds if this expression is impure, that is, its evaluation could have side effects. */
115115
predicate isImpure() { any() }
@@ -423,8 +423,6 @@ class BigIntLiteral extends @bigintliteral, Literal {
423423
* ```
424424
*/
425425
class StringLiteral extends @stringliteral, Literal {
426-
override string getStringValue() { result = getValue() }
427-
428426
/**
429427
* Gets the value of this string literal parsed as a regular expression, if possible.
430428
*
@@ -839,8 +837,6 @@ class SeqExpr extends @seqexpr, Expr {
839837

840838
override predicate isImpure() { getAnOperand().isImpure() }
841839

842-
override string getStringValue() { result = getLastOperand().getStringValue() }
843-
844840
override Expr getUnderlyingValue() { result = getLastOperand().getUnderlyingValue() }
845841
}
846842

@@ -1517,13 +1513,71 @@ class URShiftExpr extends @urshiftexpr, BinaryExpr {
15171513
*/
15181514
class AddExpr extends @addexpr, BinaryExpr {
15191515
override string getOperator() { result = "+" }
1516+
}
15201517

1521-
override string getStringValue() {
1522-
result = getLeftOperand().getStringValue() + getRightOperand().getStringValue() and
1523-
result.length() < 1000 * 1000
1524-
}
1518+
/**
1519+
* Gets the string value for the expression `e`.
1520+
* This string-value is either a constant-string, or the result from a simple string-concatenation.
1521+
*/
1522+
private string getStringValue(Expr e) {
1523+
result = getConstantString(e)
1524+
or
1525+
result = getConcatenatedString(e)
1526+
}
1527+
1528+
/**
1529+
* Gets the constant string value for the expression `e`.
1530+
*/
1531+
private string getConstantString(Expr e) {
1532+
result = getConstantString(e.getUnderlyingValue())
1533+
or
1534+
result = e.(StringLiteral).getValue()
1535+
or
1536+
exists(TemplateLiteral lit | lit = e |
1537+
// fold singletons
1538+
lit.getNumChildExpr() = 0 and
1539+
result = ""
1540+
or
1541+
e.getNumChildExpr() = 1 and
1542+
result = getConstantString(lit.getElement(0))
1543+
)
1544+
or
1545+
result = e.(TemplateElement).getValue()
1546+
}
1547+
1548+
/**
1549+
* Holds if `add` is a string-concatenation where all the transitive leafs have a constant string value.
1550+
*/
1551+
private predicate hasAllConstantLeafs(AddExpr add) {
1552+
forex(Expr leaf | leaf = getAnAddOperand*(add) and not exists(getAnAddOperand(leaf)) |
1553+
exists(getConstantString(leaf))
1554+
)
1555+
}
1556+
1557+
/**
1558+
* Gets the concatenated string for a string-concatenation `add`.
1559+
* Only has a result if `add` is not itself an operand in another string-concatenation with all constant leafs.
1560+
*/
1561+
private string getConcatenatedString(Expr add) {
1562+
result = getConcatenatedString(add.getUnderlyingValue())
1563+
or
1564+
not add = getAnAddOperand(any(AddExpr parent | hasAllConstantLeafs(parent))) and
1565+
hasAllConstantLeafs(add) and
1566+
result =
1567+
strictconcat(Expr leaf |
1568+
leaf = getAnAddOperand*(add)
1569+
|
1570+
getConstantString(leaf) order by leaf.getFirstToken().getIndex()
1571+
) and
1572+
result.length() < 1000 * 1000
15251573
}
15261574

1575+
/**
1576+
* Gets an operand from `add`.
1577+
* Is specialized to `AddExpr` such that `getAnAddOperand*(add)` can be used to get a leaf from a string-concatenation transitively.
1578+
*/
1579+
private Expr getAnAddOperand(AddExpr add) { result = add.getAnOperand().getUnderlyingValue() }
1580+
15271581
/**
15281582
* A subtraction expression.
15291583
*

‎javascript/ql/src/semmle/javascript/Templates.qll

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,6 @@ class TemplateLiteral extends Expr, @templateliteral {
5757
int getNumElement() { result = count(getAnElement()) }
5858

5959
override predicate isImpure() { getAnElement().isImpure() }
60-
61-
override string getStringValue() {
62-
// fold singletons
63-
getNumChildExpr() = 0 and
64-
result = ""
65-
or
66-
getNumChildExpr() = 1 and
67-
result = getElement(0).getStringValue()
68-
}
6960
}
7061

7162
/**
@@ -103,6 +94,4 @@ class TemplateElement extends Expr, @templateelement {
10394
string getRawValue() { literals(_, result, this) }
10495

10596
override predicate isImpure() { none() }
106-
107-
override string getStringValue() { result = getValue() }
10897
}

‎javascript/ql/test/library-tests/StringConcatenation/StringOps.expected

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ concatenation
4646
| tst.js:89:3:89:14 | x |
4747
| tst.js:89:3:89:14 | x += 'three' |
4848
| tst.js:95:7:95:30 | x.conca ... three') |
49+
| tst.js:104:11:104:23 | "foo" + "bar" |
50+
| tst.js:104:11:104:31 | "foo" + ... + value |
51+
| tst.js:105:11:105:23 | value + "foo" |
52+
| tst.js:105:11:105:31 | value + ... + "bar" |
53+
| tst.js:106:11:106:33 | "foo" + ... "baz") |
54+
| tst.js:106:20:106:32 | "bar" + "baz" |
4955
concatenationOperand
5056
| closure.js:5:1:5:37 | build(' ... 'four') |
5157
| closure.js:5:7:5:11 | 'one' |
@@ -127,6 +133,18 @@ concatenationOperand
127133
| tst.js:95:7:95:7 | x |
128134
| tst.js:95:16:95:20 | 'two' |
129135
| tst.js:95:23:95:29 | 'three' |
136+
| tst.js:104:11:104:15 | "foo" |
137+
| tst.js:104:11:104:23 | "foo" + "bar" |
138+
| tst.js:104:19:104:23 | "bar" |
139+
| tst.js:104:27:104:31 | value |
140+
| tst.js:105:11:105:15 | value |
141+
| tst.js:105:11:105:23 | value + "foo" |
142+
| tst.js:105:19:105:23 | "foo" |
143+
| tst.js:105:27:105:31 | "bar" |
144+
| tst.js:106:11:106:15 | "foo" |
145+
| tst.js:106:19:106:33 | ("bar" + "baz") |
146+
| tst.js:106:20:106:24 | "bar" |
147+
| tst.js:106:28:106:32 | "baz" |
130148
concatenationLeaf
131149
| closure.js:5:7:5:11 | 'one' |
132150
| closure.js:5:14:5:18 | 'two' |
@@ -199,6 +217,16 @@ concatenationLeaf
199217
| tst.js:95:7:95:7 | x |
200218
| tst.js:95:16:95:20 | 'two' |
201219
| tst.js:95:23:95:29 | 'three' |
220+
| tst.js:104:11:104:15 | "foo" |
221+
| tst.js:104:19:104:23 | "bar" |
222+
| tst.js:104:27:104:31 | value |
223+
| tst.js:105:11:105:15 | value |
224+
| tst.js:105:19:105:23 | "foo" |
225+
| tst.js:105:27:105:31 | "bar" |
226+
| tst.js:106:11:106:15 | "foo" |
227+
| tst.js:106:19:106:33 | ("bar" + "baz") |
228+
| tst.js:106:20:106:24 | "bar" |
229+
| tst.js:106:28:106:32 | "baz" |
202230
concatenationNode
203231
| closure.js:5:1:5:37 | build(' ... 'four') |
204232
| closure.js:5:1:5:46 | build(' ... 'five' |
@@ -318,6 +346,22 @@ concatenationNode
318346
| tst.js:95:7:95:30 | x.conca ... three') |
319347
| tst.js:95:16:95:20 | 'two' |
320348
| tst.js:95:23:95:29 | 'three' |
349+
| tst.js:104:11:104:15 | "foo" |
350+
| tst.js:104:11:104:23 | "foo" + "bar" |
351+
| tst.js:104:11:104:31 | "foo" + ... + value |
352+
| tst.js:104:19:104:23 | "bar" |
353+
| tst.js:104:27:104:31 | value |
354+
| tst.js:105:11:105:15 | value |
355+
| tst.js:105:11:105:23 | value + "foo" |
356+
| tst.js:105:11:105:31 | value + ... + "bar" |
357+
| tst.js:105:19:105:23 | "foo" |
358+
| tst.js:105:27:105:31 | "bar" |
359+
| tst.js:106:11:106:15 | "foo" |
360+
| tst.js:106:11:106:33 | "foo" + ... "baz") |
361+
| tst.js:106:19:106:33 | ("bar" + "baz") |
362+
| tst.js:106:20:106:24 | "bar" |
363+
| tst.js:106:20:106:32 | "bar" + "baz" |
364+
| tst.js:106:28:106:32 | "baz" |
321365
operand
322366
| closure.js:5:1:5:37 | build(' ... 'four') | 0 | closure.js:5:7:5:11 | 'one' |
323367
| closure.js:5:1:5:37 | build(' ... 'four') | 1 | closure.js:5:14:5:28 | 'two' + 'three' |
@@ -421,6 +465,18 @@ operand
421465
| tst.js:95:7:95:30 | x.conca ... three') | 0 | tst.js:95:7:95:7 | x |
422466
| tst.js:95:7:95:30 | x.conca ... three') | 1 | tst.js:95:16:95:20 | 'two' |
423467
| tst.js:95:7:95:30 | x.conca ... three') | 2 | tst.js:95:23:95:29 | 'three' |
468+
| tst.js:104:11:104:23 | "foo" + "bar" | 0 | tst.js:104:11:104:15 | "foo" |
469+
| tst.js:104:11:104:23 | "foo" + "bar" | 1 | tst.js:104:19:104:23 | "bar" |
470+
| tst.js:104:11:104:31 | "foo" + ... + value | 0 | tst.js:104:11:104:23 | "foo" + "bar" |
471+
| tst.js:104:11:104:31 | "foo" + ... + value | 1 | tst.js:104:27:104:31 | value |
472+
| tst.js:105:11:105:23 | value + "foo" | 0 | tst.js:105:11:105:15 | value |
473+
| tst.js:105:11:105:23 | value + "foo" | 1 | tst.js:105:19:105:23 | "foo" |
474+
| tst.js:105:11:105:31 | value + ... + "bar" | 0 | tst.js:105:11:105:23 | value + "foo" |
475+
| tst.js:105:11:105:31 | value + ... + "bar" | 1 | tst.js:105:27:105:31 | "bar" |
476+
| tst.js:106:11:106:33 | "foo" + ... "baz") | 0 | tst.js:106:11:106:15 | "foo" |
477+
| tst.js:106:11:106:33 | "foo" + ... "baz") | 1 | tst.js:106:19:106:33 | ("bar" + "baz") |
478+
| tst.js:106:20:106:32 | "bar" + "baz" | 0 | tst.js:106:20:106:24 | "bar" |
479+
| tst.js:106:20:106:32 | "bar" + "baz" | 1 | tst.js:106:28:106:32 | "baz" |
424480
nextLeaf
425481
| closure.js:5:7:5:11 | 'one' | closure.js:5:14:5:18 | 'two' |
426482
| closure.js:5:14:5:18 | 'two' | closure.js:5:22:5:28 | 'three' |
@@ -466,6 +522,12 @@ nextLeaf
466522
| tst.js:89:3:89:3 | x | tst.js:89:8:89:14 | 'three' |
467523
| tst.js:95:7:95:7 | x | tst.js:95:16:95:20 | 'two' |
468524
| tst.js:95:16:95:20 | 'two' | tst.js:95:23:95:29 | 'three' |
525+
| tst.js:104:11:104:15 | "foo" | tst.js:104:19:104:23 | "bar" |
526+
| tst.js:104:19:104:23 | "bar" | tst.js:104:27:104:31 | value |
527+
| tst.js:105:11:105:15 | value | tst.js:105:19:105:23 | "foo" |
528+
| tst.js:105:19:105:23 | "foo" | tst.js:105:27:105:31 | "bar" |
529+
| tst.js:106:11:106:15 | "foo" | tst.js:106:19:106:33 | ("bar" + "baz") |
530+
| tst.js:106:20:106:24 | "bar" | tst.js:106:28:106:32 | "baz" |
469531
htmlRoot
470532
| html-concat.js:2:14:2:26 | `<b>${x}</b>` |
471533
| html-concat.js:3:14:3:26 | `<B>${x}</B>` |
@@ -488,3 +550,13 @@ htmlLeaf
488550
| html-concat.js:8:15:10:23 | .\\n \\n ... um!</i> |
489551
| html-concat.js:13:3:13:8 | buffer |
490552
| html-concat.js:13:13:13:18 | '<li>' |
553+
getStringValue
554+
| tst.js:104:11:104:15 | "foo" | foo |
555+
| tst.js:104:11:104:23 | "foo" + "bar" | foobar |
556+
| tst.js:104:19:104:23 | "bar" | bar |
557+
| tst.js:105:19:105:23 | "foo" | foo |
558+
| tst.js:105:27:105:31 | "bar" | bar |
559+
| tst.js:106:11:106:15 | "foo" | foo |
560+
| tst.js:106:11:106:33 | "foo" + ... "baz") | foobarbaz |
561+
| tst.js:106:20:106:24 | "bar" | bar |
562+
| tst.js:106:28:106:32 | "baz" | baz |

‎javascript/ql/test/library-tests/StringConcatenation/StringOps.ql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,8 @@ query predicate nextLeaf(StringOps::ConcatenationNode node, DataFlow::Node next)
1919
query StringOps::HtmlConcatenationRoot htmlRoot() { any() }
2020

2121
query StringOps::HtmlConcatenationLeaf htmlLeaf() { any() }
22+
23+
query string getStringValue(Expr e) {
24+
result = e.getStringValue() and
25+
e.getEnclosingFunction().getName() = "stringValue"
26+
}

‎javascript/ql/test/library-tests/StringConcatenation/tst.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,9 @@ function concatCall() {
9999
function arrayConcat(a, b) {
100100
return [].concat(a, b);
101101
}
102+
103+
function stringValue() {
104+
var a = "foo" + "bar" + value;
105+
var b = value + "foo" + "bar";
106+
var c = "foo" + ("bar" + "baz")
107+
}

0 commit comments

Comments
 (0)