From f2eed4d8c4d551ddaad5d19b8847d4cbeaccdd2e Mon Sep 17 00:00:00 2001 From: Tom Hvitved Date: Thu, 24 Aug 2023 12:08:34 +0200 Subject: [PATCH] Data flow: Fix a bad join order Before ``` Evaluated relational algebra for predicate DataFlowImpl#248dabc3::MakeImpl#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Impl#DataFlow#167ac380::DataFlowMake#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Global#XSS#e59174e9::OrmTracking::Config#::C#::MkStage#Stage2#::Stage#Stage3Param#::flowThroughIntoCall#6#ffffff@0ea4e2mt with tuple counts: 1065437 ~0% {4} r1 = SCAN project#DataFlowImpl#248dabc3::MakeImpl#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Impl#DataFlow#167ac380::DataFlowMake#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Global#XSS#e59174e9::OrmTracking::Config#::C#::MkStage#Stage2#::Stage#Stage3Param#::fwdFlow#9#fffffffff#2 OUTPUT In.0, In.3, In.1, In.2 1158508760 ~0% {6} r2 = JOIN r1 WITH project#DataFlowImpl#248dabc3::MakeImpl#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Impl#DataFlow#167ac380::DataFlowMake#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Global#XSS#e59174e9::OrmTracking::Config#::C#::MkStage#Stage2#::Stage#Stage3Param#::flowIntoCallApa#6#ffffff_14023#join_rhs ON FIRST 2 OUTPUT Lhs.0, Lhs.2, Lhs.3, Rhs.2, Rhs.3, Rhs.4 {6} r3 = SELECT r2 ON In.5 != false 1158470345 ~4% {6} r4 = SCAN r3 OUTPUT In.4, In.1, In.2, In.0, In.3, In.5 {6} r5 = SELECT r2 ON In.5 = false 38415 ~0% {5} r6 = SCAN r5 OUTPUT In.2, In.0, In.1, In.3, In.4 4 ~0% {5} r7 = JOIN r6 WITH DataFlowImplCommon#f7de413b::MakeImplCommon#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Cached::TApproxFrontNil#f ON FIRST 1 OUTPUT Lhs.4, Lhs.2, Lhs.0, Lhs.1, Lhs.3 4 ~0% {6} r8 = SCAN r7 OUTPUT In.0, In.1, In.2, In.3, In.4, false 1158470349 ~4% {6} r9 = r4 UNION r8 44065 ~3% {6} r10 = JOIN r9 WITH project#DataFlowImpl#248dabc3::MakeImpl#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Impl#DataFlow#167ac380::DataFlowMake#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Global#XSS#e59174e9::OrmTracking::Config#::C#::MkStage#Stage2#::Stage#Stage3Param#::returnFlowsThrough#8#ffffffff ON FIRST 3 OUTPUT Lhs.4, Lhs.3, Lhs.0, Lhs.5, Lhs.2, Rhs.3 return r10 ``` After ``` Evaluated relational algebra for predicate DataFlowImpl#248dabc3::MakeImpl#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Impl#DataFlow#167ac380::DataFlowMake#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Global#XSS#e59174e9::OrmTracking::Config#::C#::MkStage#Stage2#::Stage#Stage3Param#::flowThroughIntoCall#6#ffffff@979c54q9 with tuple counts: 11095 ~0% {4} r1 = SCAN project#DataFlowImpl#248dabc3::MakeImpl#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Impl#DataFlow#167ac380::DataFlowMake#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Global#XSS#e59174e9::OrmTracking::Config#::C#::MkStage#Stage2#::Stage#Stage3Param#::returnFlowsThrough#8#ffffffff OUTPUT In.0, In.3, In.1, In.2 470154 ~1% {8} r2 = JOIN r1 WITH project#DataFlowImpl#248dabc3::MakeImpl#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Impl#DataFlow#167ac380::DataFlowMake#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Global#XSS#e59174e9::OrmTracking::Config#::C#::MkStage#Stage2#::Stage#Stage3Param#::flowIntoCallApa#6#ffffff_20134#join_rhs ON FIRST 1 OUTPUT Lhs.1, Lhs.0, Lhs.2, Lhs.3, Rhs.1, Rhs.2, Rhs.3, Rhs.4 {8} r3 = SELECT r2 ON In.6 != false 470152 ~0% {8} r4 = SCAN r3 OUTPUT In.5, In.2, In.3, In.7, In.0, In.1, In.4, In.6 {8} r5 = SELECT r2 ON In.6 = false 2 ~0% {7} r6 = SCAN r5 OUTPUT In.3, In.0, In.1, In.2, In.4, In.5, In.7 0 ~0% {7} r7 = JOIN r6 WITH DataFlowImplCommon#f7de413b::MakeImplCommon#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Cached::TApproxFrontNil#f ON FIRST 1 OUTPUT Lhs.5, Lhs.3, Lhs.0, Lhs.6, Lhs.1, Lhs.2, Lhs.4 0 ~0% {8} r8 = SCAN r7 OUTPUT In.0, In.1, In.2, In.3, In.4, In.5, In.6, false 470152 ~0% {8} r9 = r4 UNION r8 44065 ~3% {6} r10 = JOIN r9 WITH project#DataFlowImpl#248dabc3::MakeImpl#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Impl#DataFlow#167ac380::DataFlowMake#DataFlowImplSpecific#21008cd7::RubyDataFlow#::Global#XSS#e59174e9::OrmTracking::Config#::C#::MkStage#Stage2#::Stage#Stage3Param#::fwdFlow#9#fffffffff#2 ON FIRST 4 OUTPUT Lhs.6, Lhs.0, Lhs.5, Lhs.7, Lhs.2, Lhs.4 return r10 ``` --- shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll index 9eaae9537a37..23caaee0cce1 100644 --- a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll @@ -1623,12 +1623,11 @@ module MakeImpl { DataFlowCall call, ArgNodeEx arg, ParamNodeEx p, boolean allowsFieldFlow, Ap argAp, Ap ap ) { exists(ApApprox argApa, Typ argT | - flowIntoCallApa(call, _, pragma[only_bind_into](arg), pragma[only_bind_into](p), - allowsFieldFlow, argApa) and + returnFlowsThrough(_, _, _, _, pragma[only_bind_into](p), pragma[only_bind_into](argT), + pragma[only_bind_into](argAp), ap) and + flowIntoCallApa(call, _, pragma[only_bind_into](arg), p, allowsFieldFlow, argApa) and fwdFlow(arg, _, _, _, _, _, pragma[only_bind_into](argT), pragma[only_bind_into](argAp), argApa) and - returnFlowsThrough(_, _, _, _, p, pragma[only_bind_into](argT), - pragma[only_bind_into](argAp), ap) and if allowsFieldFlow = false then argAp instanceof ApNil else any() ) }