correctly weed out benign calls inside attributes

erik-krogh · erik-krogh · commit 67b38ed3011a · 2019-11-11T15:30:33.000+01:00
diff --git a/javascript/ql/src/Statements/UseOfReturnlessFunction.ql b/javascript/ql/src/Statements/UseOfReturnlessFunction.ql
@@ -45,7 +45,7 @@ predicate benignContext(Expr e) {
 
   or
   // weeds out calls inside HTML-attributes.
-  e.getParent() instanceof CodeInAttribute or  
+  e.getParent().(ExprStmt).getParent() instanceof CodeInAttribute or
   // and JSX-attributes.
   e = any(JSXAttribute attr).getValue() or 
   
diff --git a/javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/UseOfReturnlessFunction.expected b/javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/UseOfReturnlessFunction.expected
@@ -1,3 +1,4 @@
+| tst.html:19:28:19:40 | addHandlers() | the $@ does not return anything, yet the return value is used. | tst.html:5:7:12:7 | functio ...       } | function addHandlers |
 | tst.js:20:17:20:33 | onlySideEffects() | the $@ does not return anything, yet the return value is used. | tst.js:11:5:13:5 | functio ... )\\n    } | function onlySideEffects |
 | tst.js:24:13:24:29 | onlySideEffects() | the $@ does not return anything, yet the return value is used. | tst.js:11:5:13:5 | functio ... )\\n    } | function onlySideEffects |
 | tst.js:30:20:30:36 | onlySideEffects() | the $@ does not return anything, yet the return value is used. | tst.js:11:5:13:5 | functio ... )\\n    } | function onlySideEffects |
diff --git a/javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/tst.html b/javascript/ql/test/query-tests/Statements/UseOfReturnlessFunction/tst.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <script>
+      function addHandlers () {
+        const cdoc = document.getElementById('container').contentDocument;
+        if (cdoc) {
+          const scriptelm = cdoc.createElement('script');
+          scriptelm.src = '../handlers.js';
+          cdoc.getElementsByTagName('head')[0].appendChild(scriptelm);
+        }
+      }
+    </script>
+  </head>
+  <body>
+    <object id="container" data="editor/svg-editor.html" onload="addHandlers()"></object>
+    <a href="javascript:addHandlers()">Foo</a>
+    <div onclick="addHandlers()">Click me</div>
+    <div onclick="console.log(addHandlers() + 3)">Click me</div> <!-- NOT OK! -->
+  </body>
+</html>

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+\| tst.html:19:28:19:40 \| addHandlers() \| the $@ does not return anything, yet the return value is used. \| tst.html:5:7:12:7 \| functio ... } \| function addHandlers \|`
`1`	`2`	`\| tst.js:20:17:20:33 \| onlySideEffects() \| the $@ does not return anything, yet the return value is used. \| tst.js:11:5:13:5 \| functio ... )\\n } \| function onlySideEffects \|`
`2`	`3`	`\| tst.js:24:13:24:29 \| onlySideEffects() \| the $@ does not return anything, yet the return value is used. \| tst.js:11:5:13:5 \| functio ... )\\n } \| function onlySideEffects \|`
`3`	`4`	`\| tst.js:30:20:30:36 \| onlySideEffects() \| the $@ does not return anything, yet the return value is used. \| tst.js:11:5:13:5 \| functio ... )\\n } \| function onlySideEffects \|`