hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-01 19:55:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

use tainted-object to precisely model that plain object are fine, but their properties are not

Browse Source
This commit is contained in:
erik-krogh
2023-02-15 13:23:01 +01:00
parent 09794fa836
commit 51ddb55d7b
4 changed files with 133 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionCustomizations.qll
View File

@@ -182,16 +182,17 @@ module UnsafeHtmlConstruction {
}
/** A test for the value of `typeof x`, restricting the potential types of `x`. */
class TypeTestGuard extends TaintTracking::SanitizerGuardNode, DataFlow::ValueNode {
class TypeTestGuard extends TaintTracking::LabeledSanitizerGuardNode, DataFlow::ValueNode {
override EqualityTest astNode;
Expr operand;
boolean polarity;
TypeTestGuard() { TaintTracking::isStringTypeGuard(astNode, operand, polarity) }
override predicate sanitizes(boolean outcome, Expr e) {
override predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel lbl) {
polarity = outcome and
e = operand
e = operand and
lbl.isTaint()
}
}
}

26
javascript/ql/lib/semmle/javascript/security/dataflow/UnsafeHtmlConstructionQuery.qll
View File

@@ -7,6 +7,7 @@ import javascript
private import semmle.javascript.security.dataflow.DomBasedXssCustomizations::DomBasedXss as DomBasedXss
private import semmle.javascript.security.dataflow.UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin as UnsafeJQueryPlugin
import UnsafeHtmlConstructionCustomizations::UnsafeHtmlConstruction
import semmle.javascript.security.TaintedObject
/**
* A taint-tracking configuration for reasoning about unsafe HTML constructed from library input vulnerabilities.
@@ -14,9 +15,15 @@ import UnsafeHtmlConstructionCustomizations::UnsafeHtmlConstruction
class Configration extends TaintTracking::Configuration {
Configration() { this = "UnsafeHtmlConstruction" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
source instanceof Source and
label = [TaintedObject::label(), DataFlow::FlowLabel::taint(), DataFlow::FlowLabel::data()]
}
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
sink instanceof Sink and
label = DataFlow::FlowLabel::taint()
}
override predicate isSanitizer(DataFlow::Node node) {
super.isSanitizer(node)
@@ -36,8 +43,19 @@ class Configration extends TaintTracking::Configuration {
DataFlow::hasPathWithoutUnmatchedReturn(source, sink)
}
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
DataFlow::localFieldStep(pred, succ)
override predicate isAdditionalFlowStep(
DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
) {
DataFlow::localFieldStep(pred, succ) and
inlbl.isTaint() and
outlbl.isTaint()
or
TaintedObject::step(pred, succ, inlbl, outlbl)
or
// property read from a tainted object is considered tainted
succ.(DataFlow::PropRead).getBase() = pred and
inlbl = TaintedObject::label() and
outlbl = DataFlow::FlowLabel::taint()
}
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {

94
javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/UnsafeHtmlConstruction.expected
View File

@@ -3,15 +3,33 @@ nodes
| jquery-plugin.js:11:27:11:31 | stuff |
| jquery-plugin.js:11:34:11:40 | options |
| jquery-plugin.js:11:34:11:40 | options |
| jquery-plugin.js:11:34:11:40 | options |
| jquery-plugin.js:11:34:11:40 | options |
| jquery-plugin.js:12:31:12:37 | options |
| jquery-plugin.js:12:31:12:37 | options |
| jquery-plugin.js:12:31:12:37 | options |
| jquery-plugin.js:12:31:12:41 | options.foo |
| jquery-plugin.js:12:31:12:41 | options.foo |
| jquery-plugin.js:12:31:12:41 | options.foo |
| jquery-plugin.js:12:31:12:41 | options.foo |
| jquery-plugin.js:14:31:14:35 | stuff |
| jquery-plugin.js:14:31:14:35 | stuff |
| lib2/index.ts:1:28:1:28 | s |
| lib2/index.ts:1:28:1:28 | s |
| lib2/index.ts:2:29:2:29 | s |
| lib2/index.ts:2:29:2:29 | s |
| lib2/index.ts:2:27:2:27 | s |
| lib2/index.ts:2:27:2:27 | s |
| lib2/index.ts:6:29:6:36 | settings |
| lib2/index.ts:6:29:6:36 | settings |
| lib2/index.ts:6:29:6:36 | settings |
| lib2/index.ts:7:58:7:65 | settings |
| lib2/index.ts:7:58:7:65 | settings |
| lib2/index.ts:13:9:13:41 | name |
| lib2/index.ts:13:16:13:23 | settings |
| lib2/index.ts:13:16:13:33 | settings.mySetting |
| lib2/index.ts:13:16:13:36 | setting ... ting[i] |
| lib2/index.ts:13:16:13:41 | setting ... i].name |
| lib2/index.ts:18:62:18:65 | name |
| lib2/index.ts:18:62:18:65 | name |
| lib/src/MyNode.ts:1:28:1:28 | s |
| lib/src/MyNode.ts:1:28:1:28 | s |
| lib/src/MyNode.ts:2:29:2:29 | s |
@@ -41,13 +59,31 @@ nodes
| main.js:53:20:53:20 | s |
| main.js:56:28:56:34 | options |
| main.js:56:28:56:34 | options |
| main.js:56:28:56:34 | options |
| main.js:56:28:56:34 | options |
| main.js:57:11:59:5 | defaults |
| main.js:57:11:59:5 | defaults |
| main.js:57:11:59:5 | defaults |
| main.js:57:22:59:5 | {\\n ... "\\n } |
| main.js:57:22:59:5 | {\\n ... "\\n } |
| main.js:57:22:59:5 | {\\n ... "\\n } |
| main.js:60:11:60:48 | settings |
| main.js:60:11:60:48 | settings |
| main.js:60:11:60:48 | settings |
| main.js:60:22:60:48 | $.exten ... ptions) |
| main.js:60:22:60:48 | $.exten ... ptions) |
| main.js:60:22:60:48 | $.exten ... ptions) |
| main.js:60:31:60:38 | defaults |
| main.js:60:31:60:38 | defaults |
| main.js:60:31:60:38 | defaults |
| main.js:60:41:60:47 | options |
| main.js:60:41:60:47 | options |
| main.js:60:41:60:47 | options |
| main.js:62:19:62:26 | settings |
| main.js:62:19:62:26 | settings |
| main.js:62:19:62:26 | settings |
| main.js:62:19:62:31 | settings.name |
| main.js:62:19:62:31 | settings.name |
| main.js:62:19:62:31 | settings.name |
| main.js:62:19:62:31 | settings.name |
| main.js:66:35:66:41 | attrVal |
@@ -102,12 +138,32 @@ edges
| jquery-plugin.js:11:27:11:31 | stuff | jquery-plugin.js:14:31:14:35 | stuff |
| jquery-plugin.js:11:34:11:40 | options | jquery-plugin.js:12:31:12:37 | options |
| jquery-plugin.js:11:34:11:40 | options | jquery-plugin.js:12:31:12:37 | options |
| jquery-plugin.js:11:34:11:40 | options | jquery-plugin.js:12:31:12:37 | options |
| jquery-plugin.js:11:34:11:40 | options | jquery-plugin.js:12:31:12:37 | options |
| jquery-plugin.js:11:34:11:40 | options | jquery-plugin.js:12:31:12:37 | options |
| jquery-plugin.js:11:34:11:40 | options | jquery-plugin.js:12:31:12:37 | options |
| jquery-plugin.js:12:31:12:37 | options | jquery-plugin.js:12:31:12:41 | options.foo |
| jquery-plugin.js:12:31:12:37 | options | jquery-plugin.js:12:31:12:41 | options.foo |
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s |
| jquery-plugin.js:12:31:12:37 | options | jquery-plugin.js:12:31:12:41 | options.foo |
| jquery-plugin.js:12:31:12:37 | options | jquery-plugin.js:12:31:12:41 | options.foo |
| jquery-plugin.js:12:31:12:37 | options | jquery-plugin.js:12:31:12:41 | options.foo |
| jquery-plugin.js:12:31:12:37 | options | jquery-plugin.js:12:31:12:41 | options.foo |
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:27:2:27 | s |
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:27:2:27 | s |
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:27:2:27 | s |
| lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:27:2:27 | s |
| lib2/index.ts:6:29:6:36 | settings | lib2/index.ts:7:58:7:65 | settings |
| lib2/index.ts:6:29:6:36 | settings | lib2/index.ts:7:58:7:65 | settings |
| lib2/index.ts:6:29:6:36 | settings | lib2/index.ts:7:58:7:65 | settings |
| lib2/index.ts:6:29:6:36 | settings | lib2/index.ts:7:58:7:65 | settings |
| lib2/index.ts:6:29:6:36 | settings | lib2/index.ts:13:16:13:23 | settings |
| lib2/index.ts:6:29:6:36 | settings | lib2/index.ts:13:16:13:23 | settings |
| lib2/index.ts:13:9:13:41 | name | lib2/index.ts:18:62:18:65 | name |
| lib2/index.ts:13:9:13:41 | name | lib2/index.ts:18:62:18:65 | name |
| lib2/index.ts:13:16:13:23 | settings | lib2/index.ts:13:16:13:33 | settings.mySetting |
| lib2/index.ts:13:16:13:33 | settings.mySetting | lib2/index.ts:13:16:13:36 | setting ... ting[i] |
| lib2/index.ts:13:16:13:36 | setting ... ting[i] | lib2/index.ts:13:16:13:41 | setting ... i].name |
| lib2/index.ts:13:16:13:41 | setting ... i].name | lib2/index.ts:13:9:13:41 | name |
| lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s |
| lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s |
| lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s |
@@ -136,13 +192,35 @@ edges
| main.js:53:20:53:20 | s | main.js:41:17:41:17 | s |
| main.js:56:28:56:34 | options | main.js:60:41:60:47 | options |
| main.js:56:28:56:34 | options | main.js:60:41:60:47 | options |
| main.js:56:28:56:34 | options | main.js:60:41:60:47 | options |
| main.js:56:28:56:34 | options | main.js:60:41:60:47 | options |
| main.js:56:28:56:34 | options | main.js:60:41:60:47 | options |
| main.js:56:28:56:34 | options | main.js:60:41:60:47 | options |
| main.js:57:11:59:5 | defaults | main.js:60:31:60:38 | defaults |
| main.js:57:11:59:5 | defaults | main.js:60:31:60:38 | defaults |
| main.js:57:11:59:5 | defaults | main.js:60:31:60:38 | defaults |
| main.js:57:22:59:5 | {\\n ... "\\n } | main.js:57:11:59:5 | defaults |
| main.js:57:22:59:5 | {\\n ... "\\n } | main.js:57:11:59:5 | defaults |
| main.js:57:22:59:5 | {\\n ... "\\n } | main.js:57:11:59:5 | defaults |
| main.js:60:11:60:48 | settings | main.js:62:19:62:26 | settings |
| main.js:60:11:60:48 | settings | main.js:62:19:62:26 | settings |
| main.js:60:11:60:48 | settings | main.js:62:19:62:26 | settings |
| main.js:60:22:60:48 | $.exten ... ptions) | main.js:60:11:60:48 | settings |
| main.js:60:22:60:48 | $.exten ... ptions) | main.js:60:11:60:48 | settings |
| main.js:60:22:60:48 | $.exten ... ptions) | main.js:60:11:60:48 | settings |
| main.js:60:31:60:38 | defaults | main.js:60:22:60:48 | $.exten ... ptions) |
| main.js:60:31:60:38 | defaults | main.js:60:22:60:48 | $.exten ... ptions) |
| main.js:60:31:60:38 | defaults | main.js:60:22:60:48 | $.exten ... ptions) |
| main.js:60:41:60:47 | options | main.js:57:22:59:5 | {\\n ... "\\n } |
| main.js:60:41:60:47 | options | main.js:57:22:59:5 | {\\n ... "\\n } |
| main.js:60:41:60:47 | options | main.js:57:22:59:5 | {\\n ... "\\n } |
| main.js:60:41:60:47 | options | main.js:60:22:60:48 | $.exten ... ptions) |
| main.js:60:41:60:47 | options | main.js:60:22:60:48 | $.exten ... ptions) |
| main.js:60:41:60:47 | options | main.js:60:22:60:48 | $.exten ... ptions) |
| main.js:62:19:62:26 | settings | main.js:62:19:62:31 | settings.name |
| main.js:62:19:62:26 | settings | main.js:62:19:62:31 | settings.name |
| main.js:62:19:62:26 | settings | main.js:62:19:62:31 | settings.name |
| main.js:62:19:62:26 | settings | main.js:62:19:62:31 | settings.name |
| main.js:62:19:62:26 | settings | main.js:62:19:62:31 | settings.name |
| main.js:62:19:62:26 | settings | main.js:62:19:62:31 | settings.name |
| main.js:66:35:66:41 | attrVal | main.js:67:63:67:69 | attrVal |
@@ -199,7 +277,9 @@ edges
#select
| jquery-plugin.js:12:31:12:41 | options.foo | jquery-plugin.js:11:34:11:40 | options | jquery-plugin.js:12:31:12:41 | options.foo | This HTML construction which depends on $@ might later allow $@. | jquery-plugin.js:11:34:11:40 | options | library input | jquery-plugin.js:12:20:12:53 | "<span> ... /span>" | cross-site scripting |
| jquery-plugin.js:14:31:14:35 | stuff | jquery-plugin.js:11:27:11:31 | stuff | jquery-plugin.js:14:31:14:35 | stuff | This HTML construction which depends on $@ might later allow $@. | jquery-plugin.js:11:27:11:31 | stuff | library input | jquery-plugin.js:14:20:14:47 | "<span> ... /span>" | cross-site scripting |
| lib2/index.ts:2:29:2:29 | s | lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:29:2:29 | s | This HTML construction which depends on $@ might later allow $@. | lib2/index.ts:1:28:1:28 | s | library input | lib2/index.ts:3:49:3:52 | html | cross-site scripting |
| lib2/index.ts:2:27:2:27 | s | lib2/index.ts:1:28:1:28 | s | lib2/index.ts:2:27:2:27 | s | This HTML construction which depends on $@ might later allow $@. | lib2/index.ts:1:28:1:28 | s | library input | lib2/index.ts:3:47:3:50 | html | cross-site scripting |
| lib2/index.ts:7:58:7:65 | settings | lib2/index.ts:6:29:6:36 | settings | lib2/index.ts:7:58:7:65 | settings | This HTML construction which depends on $@ might later allow $@. | lib2/index.ts:6:29:6:36 | settings | library input | lib2/index.ts:7:47:7:77 | "<span> ... /span>" | cross-site scripting |
| lib2/index.ts:18:62:18:65 | name | lib2/index.ts:6:29:6:36 | settings | lib2/index.ts:18:62:18:65 | name | This HTML construction which depends on $@ might later allow $@. | lib2/index.ts:6:29:6:36 | settings | library input | lib2/index.ts:18:51:18:77 | "<span> ... /span>" | cross-site scripting |
| lib/src/MyNode.ts:2:29:2:29 | s | lib/src/MyNode.ts:1:28:1:28 | s | lib/src/MyNode.ts:2:29:2:29 | s | This HTML construction which depends on $@ might later allow $@. | lib/src/MyNode.ts:1:28:1:28 | s | library input | lib/src/MyNode.ts:3:49:3:52 | html | cross-site scripting |
| main.js:2:29:2:29 | s | main.js:1:55:1:55 | s | main.js:2:29:2:29 | s | This HTML construction which depends on $@ might later allow $@. | main.js:1:55:1:55 | s | library input | main.js:3:49:3:52 | html | cross-site scripting |
| main.js:7:49:7:49 | s | main.js:6:49:6:49 | s | main.js:7:49:7:49 | s | This XML parsing which depends on $@ might later allow $@. | main.js:6:49:6:49 | s | library input | main.js:8:48:8:66 | doc.documentElement | cross-site scripting |

23
javascript/ql/test/query-tests/Security/CWE-079/UnsafeHtmlConstruction/lib2/index.ts
View File

@@ -1,4 +1,21 @@
export function trivialXss(s: string) {
const html = "<span>" + s + "</span>"; // NOT OK - this file is recognized as a main file.
document.querySelector("#html").innerHTML = html;
}
const html = "<span>" + s + "</span>"; // NOT OK - this file is recognized as a main file.
document.querySelector("#html").innerHTML = html;
}
export function objectStuff(settings: any, i: number) {
document.querySelector("#html").innerHTML = "<span>" + settings + "</span>"; // NOT OK
var name;
if (settings.mySetting && settings.mySetting.length !== 0) {
for (i = 0; i < settings.mySetting.length; ++i) {
if (typeof settings.mySetting[i] === "object") {
name = settings.mySetting[i].name; // `settings.mySetting[i]` is correctly sanitized, as it is an object. However, the `name` property is stil tainted.
} else {
name = "";
}
document.querySelector("#html").innerHTML = "<span>" + name + "</span>"; // NOT OK
}
}
}