hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-21 03:06:31 +01:00
Code Issues Packages Projects Releases Wiki Activity

JS: fix FPs in UselessConditional

Browse Source
This commit is contained in:
Asger F
2018-12-13 15:26:36 +00:00
parent a4b3b1e8c8
commit ae4b55de9a
3 changed files with 22 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
javascript/ql/src/Statements/UselessConditional.ql
View File

@@ -109,15 +109,26 @@ predicate whitelist(Expr e) {
/** /**
* Holds if `e` is part of a conditional node `cond` that evaluates * Holds if `e` is part of a conditional node `cond` that evaluates
* `e` and checks its value for truthiness. * `e` and checks its value for truthiness, and the return value of `e`
* is not used for anything other than this truthiness check.
*/ */
predicate isConditional(ASTNode cond, Expr e) { predicate isExplicitConditional(ASTNode cond, Expr e) {
e = cond.(IfStmt).getCondition() or e = cond.(IfStmt).getCondition() or
e = cond.(LoopStmt).getTest() or e = cond.(LoopStmt).getTest() or
e = cond.(ConditionalExpr).getCondition() or e = cond.(ConditionalExpr).getCondition() or
e = cond.(LogicalBinaryExpr).getLeftOperand() or isExplicitConditional(_, cond) and e = cond.(Expr).getUnderlyingValue().(LogicalBinaryExpr).getAnOperand()
// Include `z` in `if (x && z)`. }
isConditional(_, cond) and e = cond.(Expr).getUnderlyingValue().(LogicalBinaryExpr).getRightOperand()
/**
* Holds if `e` is part of a conditional node `cond` that evaluates
* `e` and checks its value for truthiness.
*
* The return value of `e` may have other uses besides the truthiness check,
* but if the truthiness check is always goes one way, it still indicates an error.
*/
predicate isConditional(ASTNode cond, Expr e) {
isExplicitConditional(cond, e) or
e = cond.(LogicalBinaryExpr).getLeftOperand()
} }
from ASTNode cond, DataFlow::AnalyzedNode op, boolean cv, ASTNode sel, string msg from ASTNode cond, DataFlow::AnalyzedNode op, boolean cv, ASTNode sel, string msg

1
javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected
View File

@@ -21,6 +21,7 @@
| UselessConditional.js:101:18:101:18 | x | This use of variable 'x' always evaluates to false. | | UselessConditional.js:101:18:101:18 | x | This use of variable 'x' always evaluates to false. |
| UselessConditional.js:102:19:102:19 | x | This use of variable 'x' always evaluates to false. | | UselessConditional.js:102:19:102:19 | x | This use of variable 'x' always evaluates to false. |
| UselessConditional.js:103:23:103:23 | x | This use of variable 'x' always evaluates to false. | | UselessConditional.js:103:23:103:23 | x | This use of variable 'x' always evaluates to false. |
| UselessConditional.js:109:15:109:16 | {} | This expression always evaluates to true. |
| UselessConditionalGood.js:58:12:58:13 | x2 | This use of variable 'x2' always evaluates to false. | | UselessConditionalGood.js:58:12:58:13 | x2 | This use of variable 'x2' always evaluates to false. |
| UselessConditionalGood.js:69:12:69:13 | xy | This use of variable 'xy' always evaluates to false. | | UselessConditionalGood.js:69:12:69:13 | xy | This use of variable 'xy' always evaluates to false. |
| UselessConditionalGood.js:85:12:85:13 | xy | This use of variable 'xy' always evaluates to false. | | UselessConditionalGood.js:85:12:85:13 | xy | This use of variable 'xy' always evaluates to false. |

5
javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.js
View File

@@ -104,4 +104,9 @@ async function awaitFlow(){
} }
}); });
(function(x,y) {
let obj = (x && {}) || y; // OK
if ((x && {}) || y) {} // NOT OK
});
// semmle-extractor-options: --experimental // semmle-extractor-options: --experimental