From 9651a0bfc4a802424253fedbc2cf6d0582078da7 Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Thu, 21 Nov 2019 22:58:36 -0800 Subject: [PATCH] Use the split taint predicate to emulate taint where required In particular, the OpenUrlRedirect and CleartextLogging queries, which both have taint flow into an object when one of its fields is written. --- ql/src/semmle/go/security/CleartextLogging.qll | 15 ++------------- ql/src/semmle/go/security/OpenUrlRedirect.qll | 14 ++------------ 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/ql/src/semmle/go/security/CleartextLogging.qll b/ql/src/semmle/go/security/CleartextLogging.qll index a93ac5f2ec9..0ed60a67e25 100644 --- a/ql/src/semmle/go/security/CleartextLogging.qll +++ b/ql/src/semmle/go/security/CleartextLogging.qll @@ -34,19 +34,8 @@ module CleartextLogging { // A taint propagating data-flow edge through structs: a tainted write taints the entire struct. exists(Write write | write.writesField(trg.getASuccessor*(), _, src)) or - trg.(DataFlow::BinaryOperationNode).getOperator() = "+" and - src = trg.(DataFlow::BinaryOperationNode).getAnOperand() - or - // Allow flow through functions that are considered for taint flow. - exists( - TaintTracking::FunctionModel m, DataFlow::CallNode c, DataFlow::FunctionInput inp, - DataFlow::FunctionOutput outp - | - c = m.getACall() and - m.hasTaintFlow(inp, outp) and - src = inp.getNode(c) and - trg = outp.getNode(c) - ) + // taint steps that do not include flow through fields + TaintTracking::taintStep(src, trg) and not TaintTracking::fieldReadStep(src, trg) } } } diff --git a/ql/src/semmle/go/security/OpenUrlRedirect.qll b/ql/src/semmle/go/security/OpenUrlRedirect.qll index 0447882f942..53fac4e9311 100644 --- a/ql/src/semmle/go/security/OpenUrlRedirect.qll +++ b/ql/src/semmle/go/security/OpenUrlRedirect.qll @@ -35,18 +35,8 @@ module OpenUrlRedirect { var.getType().hasQualifiedName("net/url", "URL") ) or - StringConcatenation::taintStep(pred, succ) - or - // Allow flow through functions that are considered for taint flow. - exists( - TaintTracking::FunctionModel m, DataFlow::CallNode c, DataFlow::FunctionInput inp, - DataFlow::FunctionOutput outp - | - c = m.getACall() and - m.hasTaintFlow(inp, outp) and - pred = inp.getNode(c) and - succ = outp.getNode(c) - ) + // taint steps that do not include flow through fields + TaintTracking::taintStep(pred, succ) and not TaintTracking::fieldReadStep(pred, succ) } override predicate isBarrierOut(DataFlow::Node node) { hostnameSanitizingPrefixEdge(node, _) }