Merge pull request #48 from xiemaisi/js/webview-sinks

semmle-qlci · web-flow · commit 3d0748c542d4 · 2018-08-13T09:37:33.000+01:00
Approved by asger-semmle
diff --git a/change-notes/1.18/analysis-javascript.md b/change-notes/1.18/analysis-javascript.md
@@ -44,6 +44,7 @@
   - [object.assign](https://github.com/ljharb/object.assign)
   - [object.defaults](https://github.com/jonschlinkert/object.defaults)
   - [parse-json](https://github.com/sindresorhus/parse-json)
+  - [React Native](https://facebook.github.io/react-native/)
   - [q](http://documentup.com/kriskowal/q/)
   - [ramda](https://ramdajs.com)
   - [safe-json-parse](https://github.com/Raynos/safe-json-parse)
diff --git a/javascript/ql/src/javascript.qll b/javascript/ql/src/javascript.qll
@@ -62,6 +62,7 @@ import semmle.javascript.frameworks.HttpFrameworks
 import semmle.javascript.frameworks.NoSQL
 import semmle.javascript.frameworks.PkgCloud
 import semmle.javascript.frameworks.React
+import semmle.javascript.frameworks.ReactNative
 import semmle.javascript.frameworks.Request
 import semmle.javascript.frameworks.SQL
 import semmle.javascript.frameworks.UriLibraries
diff --git a/javascript/ql/src/semmle/javascript/frameworks/ReactNative.qll b/javascript/ql/src/semmle/javascript/frameworks/ReactNative.qll
@@ -0,0 +1,16 @@
+/**
+ * Provides classes for working with [React Native](https://facebook.github.io/react-native) code.
+ */
+
+import javascript
+
+module ReactNative {
+  /** A `WebView` JSX element. */
+  class WebViewElement extends DataFlow::ValueNode, DataFlow::DefaultSourceNode {
+    override JSXElement astNode;
+
+    WebViewElement() {
+      DataFlow::moduleMember("react-native", "WebView").flowsToExpr(astNode.getNameExpr())
+    }
+  }
+}
diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/CodeInjection.qll b/javascript/ql/src/semmle/javascript/security/dataflow/CodeInjection.qll
@@ -115,6 +115,21 @@ module CodeInjection {
       )
     }
   }
+
+  /**
+   * An expression which is injected as JavaScript into a React Native `WebView`.
+   */
+  class WebViewInjectedJavaScriptSink extends Sink {
+    WebViewInjectedJavaScriptSink() {
+      exists (ReactNative::WebViewElement webView |
+        // `injectedJavaScript` property of React Native `WebView`
+        this = webView.getAPropertyWrite("injectedJavaScript").getRhs()
+        or
+        // argument to `injectJavascript` method of React Native `WebView`
+        this = webView.getAMethodCall("injectJavaScript").getArgument(0)
+      )
+    }
+  }
 }
 
 /** DEPRECATED: Use `CodeInjection::Source` instead. */
diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll b/javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll
@@ -117,6 +117,13 @@ module DomBasedXss {
         pw.interpretsValueAsHTML() and
         this = DataFlow::valueNode(pw.getRhs())
       )
+      or
+      // `html` or `source.html` properties of React Native `WebView`
+      exists (ReactNative::WebViewElement webView, DataFlow::SourceNode source |
+        source = webView or
+        source = webView.getAPropertyWrite("source").getRhs().getALocalSource() |
+        this = source.getAPropertyWrite("html").getRhs()
+      )
     }
   }
 
diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/ServerSideUrlRedirect.qll b/javascript/ql/src/semmle/javascript/security/dataflow/ServerSideUrlRedirect.qll
@@ -140,6 +140,20 @@ module ServerSideUrlRedirect {
       outcome = true
     }
   }
+
+  /**
+   * A URL attribute for a React Native `WebView`.
+   */
+  class WebViewUrlSink extends Sink {
+    WebViewUrlSink() {
+      // `url` or `source.uri` properties of React Native `WebView`
+      exists (ReactNative::WebViewElement webView, DataFlow::SourceNode source, string prop |
+        source = webView and prop = "url" or
+        source = webView.getAPropertyWrite("source").getRhs().getALocalSource() and prop = "uri" |
+        this = source.getAPropertyWrite(prop).getRhs()
+      )
+    }
+  }
 }
 
 /** DEPRECATED: Use `ServerSideUrlRedirect::Source` instead. */
diff --git a/javascript/ql/test/library-tests/frameworks/ReactNative/WebView.expected b/javascript/ql/test/library-tests/frameworks/ReactNative/WebView.expected
@@ -0,0 +1 @@
+| webview.js:6:12:6:56 | <WebVie ... om'}}/> |
diff --git a/javascript/ql/test/library-tests/frameworks/ReactNative/WebView.ql b/javascript/ql/test/library-tests/frameworks/ReactNative/WebView.ql
@@ -0,0 +1,4 @@
+import javascript
+
+from ReactNative::WebViewElement wv
+select wv
diff --git a/javascript/ql/test/library-tests/frameworks/ReactNative/webview.js b/javascript/ql/test/library-tests/frameworks/ReactNative/webview.js
@@ -0,0 +1,8 @@
+import { Component } from 'react';
+import { WebView } from 'react-native';
+
+class LgtmView extends Component {
+  render() {
+    return <WebView source={{uri: 'https://lgtm.com'}}/>;
+  }
+}
diff --git a/javascript/ql/test/query-tests/Security/CWE-079/Xss.expected b/javascript/ql/test/query-tests/Security/CWE-079/Xss.expected
@@ -1,6 +1,8 @@
 | jquery.js:4:5:4:11 | tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:33 | document.location | user-provided value |
 | jquery.js:7:5:7:34 | "<div i ... + "\\">" | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:33 | document.location | user-provided value |
 | jquery.js:8:18:8:34 | "XSS: " + tainted | Cross-site scripting vulnerability due to $@. | jquery.js:2:17:2:33 | document.location | user-provided value |
+| react-native.js:8:18:8:24 | tainted | Cross-site scripting vulnerability due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value |
+| react-native.js:9:27:9:33 | tainted | Cross-site scripting vulnerability due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value |
 | string-manipulations.js:3:16:3:32 | document.location | Cross-site scripting vulnerability due to $@. | string-manipulations.js:3:16:3:32 | document.location | user-provided value |
 | string-manipulations.js:4:16:4:37 | documen ... on.href | Cross-site scripting vulnerability due to $@. | string-manipulations.js:4:16:4:32 | document.location | user-provided value |
 | string-manipulations.js:5:16:5:47 | documen ... lueOf() | Cross-site scripting vulnerability due to $@. | string-manipulations.js:5:16:5:32 | document.location | user-provided value |
diff --git a/javascript/ql/test/query-tests/Security/CWE-079/react-native.js b/javascript/ql/test/query-tests/Security/CWE-079/react-native.js
@@ -0,0 +1,10 @@
+import express from 'express';
+import { WebView } from 'react-native';
+
+var app = express();
+
+app.get('/some/path', function(req, res) {
+  let tainted = req.param("code");
+  <WebView html={tainted}/>;            // NOT OK
+  <WebView source={{html: tainted}}/>;  // NOT OK
+});
diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection.expected b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection.expected
@@ -15,6 +15,8 @@
 | express.js:7:24:7:69 | "return ...  + "];" | $@ flows to here and is interpreted as code. | express.js:7:44:7:62 | req.param("wobble") | User-provided value |
 | express.js:9:34:9:79 | "return ...  + "];" | $@ flows to here and is interpreted as code. | express.js:9:54:9:72 | req.param("wobble") | User-provided value |
 | express.js:12:8:12:53 | "return ...  + "];" | $@ flows to here and is interpreted as code. | express.js:12:28:12:46 | req.param("wobble") | User-provided value |
+| react-native.js:8:32:8:38 | tainted | $@ flows to here and is interpreted as code. | react-native.js:7:17:7:33 | req.param("code") | User-provided value |
+| react-native.js:10:23:10:29 | tainted | $@ flows to here and is interpreted as code. | react-native.js:7:17:7:33 | req.param("code") | User-provided value |
 | tst.js:2:6:2:83 | documen ... t=")+8) | $@ flows to here and is interpreted as code. | tst.js:2:6:2:22 | document.location | User-provided value |
 | tst.js:5:12:5:33 | documen ... on.hash | $@ flows to here and is interpreted as code. | tst.js:5:12:5:28 | document.location | User-provided value |
 | tst.js:14:10:14:65 | documen ... , "$1") | $@ flows to here and is interpreted as code. | tst.js:14:10:14:24 | document.cookie | User-provided value |
diff --git a/javascript/ql/test/query-tests/Security/CWE-094/react-native.js b/javascript/ql/test/query-tests/Security/CWE-094/react-native.js
@@ -0,0 +1,11 @@
+import express from 'express';
+import { WebView } from 'react-native';
+
+var app = express();
+
+app.get('/some/path', function(req, res) {
+  let tainted = req.param("code");
+  <WebView injectedJavaScript={tainted}/>;  // NOT OK
+  let wv = <WebView/>;
+  wv.injectJavaScript(tainted);             // NOT OK
+});
diff --git a/javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/ServerSideUrlRedirect.expected b/javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/ServerSideUrlRedirect.expected
@@ -9,3 +9,5 @@
 | node.js:7:34:7:39 | target | Untrusted URL redirection due to $@. | node.js:6:26:6:32 | req.url | user-provided value |
 | node.js:15:34:15:45 | '/' + target | Untrusted URL redirection due to $@. | node.js:11:26:11:32 | req.url | user-provided value |
 | node.js:32:34:32:55 | target  ... =" + me | Untrusted URL redirection due to $@. | node.js:29:26:29:32 | req.url | user-provided value |
+| react-native.js:8:17:8:23 | tainted | Untrusted URL redirection due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value |
+| react-native.js:9:26:9:32 | tainted | Untrusted URL redirection due to $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value |
diff --git a/javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/react-native.js b/javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/react-native.js
@@ -0,0 +1,10 @@
+import express from 'express';
+import { WebView } from 'react-native';
+
+var app = express();
+
+app.get('/some/path', function(req, res) {
+  let tainted = req.param("code");
+  <WebView url={tainted}/>;            // NOT OK
+  <WebView source={{uri: tainted}}/>;  // NOT OK
+});

Original file line number	Diff line number	Diff line change
`@@ -115,6 +115,21 @@ module CodeInjection {`
`115`	`115`	`)`
`116`	`116`	`}`
`117`	`117`	`}`
	`118`	`+`
	`119`	`+ /**`
	`120`	+ * An expression which is injected as JavaScript into a React Native `WebView`.
	`121`	`+ */`
	`122`	`+ class WebViewInjectedJavaScriptSink extends Sink {`
	`123`	`+ WebViewInjectedJavaScriptSink() {`
	`124`	`+ exists (ReactNative::WebViewElement webView \|`
	`125`	+ // `injectedJavaScript` property of React Native `WebView`
	`126`	`+ this = webView.getAPropertyWrite("injectedJavaScript").getRhs()`
	`127`	`+ or`
	`128`	+ // argument to `injectJavascript` method of React Native `WebView`
	`129`	`+ this = webView.getAMethodCall("injectJavaScript").getArgument(0)`
	`130`	`+ )`
	`131`	`+ }`
	`132`	`+ }`
`118`	`133`	`}`
`119`	`134`
`120`	`135`	/** DEPRECATED: Use `CodeInjection::Source` instead. */
Original file line number	Diff line number	Diff line change
`@@ -117,6 +117,13 @@ module DomBasedXss {`
`117`	`117`	`pw.interpretsValueAsHTML() and`
`118`	`118`	`this = DataFlow::valueNode(pw.getRhs())`
`119`	`119`	`)`
	`120`	`+ or`
	`121`	+ // `html` or `source.html` properties of React Native `WebView`
	`122`	`+ exists (ReactNative::WebViewElement webView, DataFlow::SourceNode source \|`
	`123`	`+ source = webView or`
	`124`	`+ source = webView.getAPropertyWrite("source").getRhs().getALocalSource() \|`
	`125`	`+ this = source.getAPropertyWrite("html").getRhs()`
	`126`	`+ )`
`120`	`127`	`}`
`121`	`128`	`}`
`122`	`129`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+\| webview.js:6:12:6:56 \| <WebVie ... om'}}/> \|`