hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-24 16:25:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

JS: Enhance isDomProperty to check for getAPropertyRead on DOM nodes

Browse Source
This commit is contained in:
Napalys Klicius
2025-05-26 11:43:37 +02:00
parent 9b2ef8be10
commit bca1bc7153
3 changed files with 20 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
javascript/ql/lib/Expressions/ExprHasNoEffect.qll
View File

@@ -129,6 +129,20 @@ predicate noSideEffects(Expr e) {
)
}
/**
* Holds if `e` is a compound expression that may contain sub-expressions with side effects.
* We should not flag these directly as useless since we want to flag only the innermost
* expressions that actually have no effect.
*/
predicate isCompoundExpression(Expr e) {
e instanceof LogicalBinaryExpr
or
e instanceof SeqExpr
or
e instanceof ParExpr and
not e.stripParens() instanceof FunctionExpr
}
/**
* Holds if the expression `e` should be reported as having no effect.
*/
@@ -145,6 +159,7 @@ predicate hasNoEffect(Expr e) {
not isDeclaration(e) and
// exclude DOM properties, which sometimes have magical auto-update properties
not isDomProperty(e.(PropAccess).getPropertyName()) and
not isCompoundExpression(e) and
// exclude xUnit.js annotations
not e instanceof XUnitAnnotation and
// exclude common patterns that are most likely intentional

7
javascript/ql/test/query-tests/Expressions/ExprHasNoEffect/ExprHasNoEffect.expected
View File

@@ -1,11 +1,4 @@
| dom.js:2:5:2:30 | a.clien ... ientTop | This expression has no effect. |
| dom.js:2:5:2:50 | a.clien ... === !0 | This expression has no effect. |
| dom.js:2:33:2:50 | a.clientTop === !0 | This expression has no effect. |
| dom.js:3:5:3:20 | a && a.clientTop | This expression has no effect. |
| dom.js:4:5:4:28 | a.clien ... ientTop | This expression has no effect. |
| dom.js:5:18:5:43 | a.clien ... ientTop | This expression has no effect. |
| dom.js:6:18:6:63 | b && (b ... entTop) | This expression has no effect. |
| dom.js:6:23:6:63 | (b.clie ... entTop) | This expression has no effect. |
| try.js:22:9:22:26 | x.ordinaryProperty | This expression has no effect. |
| tst2.js:2:4:2:4 | 0 | This expression has no effect. |
| tst.js:3:1:3:2 | 23 | This expression has no effect. |

10
javascript/ql/test/query-tests/Expressions/ExprHasNoEffect/dom.js
View File

@@ -1,7 +1,7 @@
function f(){
a.clientTop && a.clientTop, a.clientTop === !0; // $Alert
a && a.clientTop; // $SPURIOUS:Alert
a.clientTop, a.clientTop; // $SPURIOUS:Alert
if(a) return a.clientTop && a.clientTop, a.clientTop === !0 // $SPURIOUS:Alert
if(b) return b && (b.clientTop, b.clientTop && b.clientTop), null // $SPURIOUS:Alert
a.clientTop && a.clientTop, a.clientTop === !0; //$Alert
a && a.clientTop;
a.clientTop, a.clientTop;
if(a) return a.clientTop && a.clientTop, a.clientTop === !0;
if(b) return b && (b.clientTop, b.clientTop && b.clientTop), null;
}