From f7f88689c4bbd6993d6ef3dd6856cfcdcc5a0b32 Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Tue, 22 Dec 2020 13:26:49 +0100 Subject: [PATCH] use strings in isTypeofGard --- .../Security/CWE-915/PrototypePollutingFunction.ql | 12 ++++++------ .../src/semmle/javascript/DefensiveProgramming.qll | 10 +++++----- .../src/semmle/javascript/dataflow/TaintTracking.qll | 8 ++++---- .../src/semmle/javascript/security/TaintedObject.qll | 6 +++--- .../dataflow/PrototypePollutingAssignment.qll | 6 +++--- .../dataflow/UnsafeJQueryPluginCustomizations.qll | 2 +- .../UnsafeShellCommandConstructionCustomizations.qll | 5 +---- .../UnvalidatedDynamicMethodCallCustomizations.qll | 2 +- .../javascript/security/dataflow/XssThroughDom.qll | 6 +++--- 9 files changed, 27 insertions(+), 30 deletions(-) diff --git a/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql b/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql index 3aec2c4bd8f..2ca0fdb2724 100644 --- a/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql +++ b/javascript/ql/src/Security/CWE-915/PrototypePollutingFunction.ql @@ -396,18 +396,18 @@ class InstanceOfGuard extends DataFlow::LabeledBarrierGuardNode, DataFlow::Value class TypeofGuard extends DataFlow::LabeledBarrierGuardNode, DataFlow::ValueNode { override EqualityTest astNode; Expr operand; - InferredType type; + TypeofTag tag; - TypeofGuard() { TaintTracking::isTypeofGuard(astNode, operand, type) } + TypeofGuard() { TaintTracking::isTypeofGuard(astNode, operand, tag) } override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel label) { e = operand and outcome = astNode.getPolarity() and ( - type = TTObject() and + tag = "object" and label = "constructor" or - type = TTFunction() and + tag = "function" and label = "__proto__" ) or @@ -416,10 +416,10 @@ class TypeofGuard extends DataFlow::LabeledBarrierGuardNode, DataFlow::ValueNode ( // If something is not an object, sanitize object, as both must end // in non-function prototype object. - type = TTObject() and + tag = "object" and label instanceof UnsafePropLabel or - type = TTFunction() and + tag = "function" and label = "constructor" ) } diff --git a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll index b10ba0bc22d..f4fc871be7d 100644 --- a/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll +++ b/javascript/ql/src/semmle/javascript/DefensiveProgramming.qll @@ -326,15 +326,15 @@ module DefensiveExpressionTest { */ private class TypeofTest extends EqualityTest { Expr operand; - InferredType type; + TypeofTag tag; - TypeofTest() { TaintTracking::isTypeofGuard(this, operand, type) } + TypeofTest() { TaintTracking::isTypeofGuard(this, operand, tag) } boolean getTheTestResult() { exists(boolean testResult | - testResult = true and operand.analyze().getTheType() = type + testResult = true and operand.analyze().getTheType().getTypeofTag() = tag or - testResult = false and not operand.analyze().getAType() = type + testResult = false and not operand.analyze().getAType().getTypeofTag() = tag | if getPolarity() = true then result = testResult else result = testResult.booleanNot() ) @@ -348,7 +348,7 @@ module DefensiveExpressionTest { /** * Gets the `typeof` tag that is tested. */ - TypeofTag getTag() { result = type.getTypeofTag() } + TypeofTag getTag() { result = tag } } /** diff --git a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll index a3b246d6934..204524a0173 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll @@ -899,7 +899,7 @@ module TaintTracking { Expr x; override EqualityTest astNode; - TypeOfUndefinedSanitizer() { isTypeofGuard(astNode, x, TTUndefined()) } + TypeOfUndefinedSanitizer() { isTypeofGuard(astNode, x, "undefined") } override predicate sanitizes(boolean outcome, Expr e) { outcome = astNode.getPolarity() and @@ -910,13 +910,13 @@ module TaintTracking { } /** - * Holds if `test` is a guard that checks if `operand` is typeof `type`. + * Holds if `test` is a guard that checks if `operand` is typeof `tag`. * * See `TypeOfUndefinedSanitizer` for example usage. */ - predicate isTypeofGuard(EqualityTest test, Expr operand, InferredType type) { + predicate isTypeofGuard(EqualityTest test, Expr operand, TypeofTag tag) { exists(Expr str, TypeofExpr typeof | test.hasOperands(str, typeof) | - str.mayHaveStringValue(type.getTypeofTag()) and + str.mayHaveStringValue(tag) and typeof.getOperand() = operand ) } diff --git a/javascript/ql/src/semmle/javascript/security/TaintedObject.qll b/javascript/ql/src/semmle/javascript/security/TaintedObject.qll index a59e8b8ea77..7f66594254f 100644 --- a/javascript/ql/src/semmle/javascript/security/TaintedObject.qll +++ b/javascript/ql/src/semmle/javascript/security/TaintedObject.qll @@ -103,13 +103,13 @@ module TaintedObject { boolean polarity; TypeTestGuard() { - exists(InferredType type | TaintTracking::isTypeofGuard(astNode, operand, type) | + exists(TypeofTag tag | TaintTracking::isTypeofGuard(astNode, operand, tag) | // typeof x === "object" sanitizes `x` when it evaluates to false - type = TTObject() and + tag = "object" and polarity = astNode.getPolarity().booleanNot() or // typeof x === "string" sanitizes `x` when it evaluates to true - type != TTObject() and + tag != "object" and polarity = astNode.getPolarity() ) } diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/PrototypePollutingAssignment.qll b/javascript/ql/src/semmle/javascript/security/dataflow/PrototypePollutingAssignment.qll index 2b6e413fd21..92f61f4ae8f 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/PrototypePollutingAssignment.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/PrototypePollutingAssignment.qll @@ -168,10 +168,10 @@ module PrototypePollutingAssignment { boolean polarity; TypeofCheck() { - exists(InferredType type | TaintTracking::isTypeofGuard(astNode, operand, type) | - type = TTObject() and polarity = astNode.getPolarity().booleanNot() + exists(TypeofTag value | TaintTracking::isTypeofGuard(astNode, operand, value) | + value = "object" and polarity = astNode.getPolarity().booleanNot() or - type != TTObject() and polarity = astNode.getPolarity() + value != "object" and polarity = astNode.getPolarity() ) } diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll index 6ec7241f852..94d0492c93e 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPluginCustomizations.qll @@ -134,7 +134,7 @@ module UnsafeJQueryPlugin { SyntacticConstants::isUndefined(undef) ) or - TaintTracking::isTypeofGuard(test, read.asExpr(), TTUndefined()) + TaintTracking::isTypeofGuard(test, read.asExpr(), "undefined") ) or polarity = true and diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll index f0be144cee6..9d72ba0f756 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/UnsafeShellCommandConstructionCustomizations.qll @@ -199,10 +199,7 @@ module UnsafeShellCommandConstruction { Expr x; override EqualityTest astNode; - TypeOfSanitizer() { - TaintTracking::isTypeofGuard(astNode, x, - any(InferredType t | t = TTNumber() or t = TTBoolean())) - } + TypeOfSanitizer() { TaintTracking::isTypeofGuard(astNode, x, ["number", "boolean"]) } override predicate sanitizes(boolean outcome, Expr e) { outcome = astNode.getPolarity() and diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll index c3283a12cee..8630642e774 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/UnvalidatedDynamicMethodCallCustomizations.qll @@ -100,7 +100,7 @@ module UnvalidatedDynamicMethodCall { override EqualityTest astNode; Expr operand; - FunctionCheck() { TaintTracking::isTypeofGuard(astNode, operand, TTFunction()) } + FunctionCheck() { TaintTracking::isTypeofGuard(astNode, operand, "function") } override predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) { outcome = astNode.getPolarity() and diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/XssThroughDom.qll b/javascript/ql/src/semmle/javascript/security/dataflow/XssThroughDom.qll index 0c807d3f539..8c3157fcce4 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/XssThroughDom.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/XssThroughDom.qll @@ -108,13 +108,13 @@ module XssThroughDom { boolean polarity; TypeTestGuard() { - exists(InferredType type | TaintTracking::isTypeofGuard(astNode, operand, type) | + exists(TypeofTag tag | TaintTracking::isTypeofGuard(astNode, operand, tag) | // typeof x === "string" sanitizes `x` when it evaluates to false - type = TTString() and + tag = "string" and polarity = astNode.getPolarity().booleanNot() or // typeof x === "object" sanitizes `x` when it evaluates to true - type != TTString() and + tag != "string" and polarity = astNode.getPolarity() ) }