Skip to content

Commit 6f54bb1

Browse files
committed
only calculate getStringValue for concatenation roots
1 parent ef109d9 commit 6f54bb1

File tree

2 files changed

+56
-20
lines changed

2 files changed

+56
-20
lines changed

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

Lines changed: 56 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,64 @@ 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+
* Gets the concatenated string for a string-concatenation `add`.
1550+
* Only has a result if `add` is not itself an operand in another string-concatenation.
1551+
*/
1552+
private string getConcatenatedString(Expr add) {
1553+
result = getConcatenatedString(add.getUnderlyingValue())
1554+
or
1555+
not add = getAnAddOperand(_) and
1556+
forex(Expr leaf | leaf = getAnAddOperand*(add) and not exists(getAnAddOperand(leaf)) |
1557+
exists(getConstantString(leaf))
1558+
) and
1559+
result =
1560+
concat(Expr leaf |
1561+
leaf = getAnAddOperand*(add)
1562+
|
1563+
getConstantString(leaf) order by leaf.getFirstToken().getIndex()
1564+
) and
1565+
result.length() < 1000 * 1000
15251566
}
15261567

1568+
/**
1569+
* Gets an operand from `add`.
1570+
* Is specialized to `AddExpr` such that `getAnAddOperand*(add)` can be used to get a leaf from a string-concatenation transitively.
1571+
*/
1572+
private Expr getAnAddOperand(AddExpr add) { result = add.getAnOperand() }
1573+
15271574
/**
15281575
* A subtraction expression.
15291576
*

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
}

0 commit comments

Comments
 (0)