From d3ee6583992e486bd4d7334ab8b5d50a876b9cfe Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 20 Dec 2024 11:02:40 +0100 Subject: [PATCH] Python: resolve remaining TODOs --- .../security/dataflow/LdapInjectionQuery.qll | 14 ++----------- .../security/dataflow/RegexInjectionQuery.qll | 10 +++++---- .../ServerSideRequestForgeryQuery.qll | 21 +++++++++++-------- .../UnsafeShellCommandConstructionQuery.qll | 13 +++++++----- .../WeakSensitiveDataHashingQuery.qll | 12 ++--------- .../CWE-020-ExternalAPIs/ExternalAPIs.qll | 5 +---- .../src/Security/CWE-327/FluentApiModel.qll | 4 +--- .../semmle/python/libraries/SmtpLib.qll | 4 +--- 8 files changed, 33 insertions(+), 50 deletions(-) diff --git a/python/ql/lib/semmle/python/security/dataflow/LdapInjectionQuery.qll b/python/ql/lib/semmle/python/security/dataflow/LdapInjectionQuery.qll index a610f229844e..7d0f5da6a5a5 100644 --- a/python/ql/lib/semmle/python/security/dataflow/LdapInjectionQuery.qll +++ b/python/ql/lib/semmle/python/security/dataflow/LdapInjectionQuery.qll @@ -20,12 +20,7 @@ private module LdapInjectionDnConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof DnSanitizer } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/Security/CWE-090/LdapInjection.ql:26: Column 1 does not select a source or sink originating from the flow call on line 21 - // ql/src/Security/CWE-090/LdapInjection.ql:27: Column 5 does not select a source or sink originating from the flow call on line 21 - none() - } + predicate observeDiffInformedIncrementalMode() { any() } } /** Global taint-tracking for detecting "LDAP injection via the distinguished name (DN) parameter" vulnerabilities. */ @@ -38,12 +33,7 @@ private module LdapInjectionFilterConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof FilterSanitizer } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/Security/CWE-090/LdapInjection.ql:26: Column 1 does not select a source or sink originating from the flow call on line 24 - // ql/src/Security/CWE-090/LdapInjection.ql:27: Column 5 does not select a source or sink originating from the flow call on line 24 - none() - } + predicate observeDiffInformedIncrementalMode() { any() } } /** Global taint-tracking for detecting "LDAP injection via the filter parameter" vulnerabilities. */ diff --git a/python/ql/lib/semmle/python/security/dataflow/RegexInjectionQuery.qll b/python/ql/lib/semmle/python/security/dataflow/RegexInjectionQuery.qll index 1d55f592d3f6..b7e234fd6cb4 100644 --- a/python/ql/lib/semmle/python/security/dataflow/RegexInjectionQuery.qll +++ b/python/ql/lib/semmle/python/security/dataflow/RegexInjectionQuery.qll @@ -19,10 +19,12 @@ private module RegexInjectionConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/Security/CWE-730/RegexInjection.ql:29: Column 7 selects sink.getRegexExecution - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(Sink).getLocation() + or + result = sink.(Sink).getRegexExecution().getLocation() } } diff --git a/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryQuery.qll b/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryQuery.qll index 28173eb1328b..a9d6c6a99b02 100644 --- a/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryQuery.qll +++ b/python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryQuery.qll @@ -30,11 +30,12 @@ private module FullServerSideRequestForgeryConfig implements DataFlow::ConfigSig node instanceof FullUrlControlSanitizer } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryQuery.qll:47: Flow call outside 'select' clause - // ql/src/Security/CWE-918/FullServerSideRequestForgery.ql:24: Column 1 selects sink.getRequest - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(Sink).getLocation() + or + result = sink.(Sink).getRequest().getLocation() } } @@ -66,10 +67,12 @@ private module PartialServerSideRequestForgeryConfig implements DataFlow::Config predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/Security/CWE-918/PartialServerSideRequestForgery.ql:24: Column 1 selects sink.getRequest - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(Sink).getLocation() + or + result = sink.(Sink).getRequest().getLocation() } } diff --git a/python/ql/lib/semmle/python/security/dataflow/UnsafeShellCommandConstructionQuery.qll b/python/ql/lib/semmle/python/security/dataflow/UnsafeShellCommandConstructionQuery.qll index 89de8e6961fe..7ac03b3aa8e3 100644 --- a/python/ql/lib/semmle/python/security/dataflow/UnsafeShellCommandConstructionQuery.qll +++ b/python/ql/lib/semmle/python/security/dataflow/UnsafeShellCommandConstructionQuery.qll @@ -29,11 +29,14 @@ module UnsafeShellCommandConstructionConfig implements DataFlow::ConfigSig { // override to require the path doesn't have unmatched return steps DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/Security/CWE-078/UnsafeShellCommandConstruction.ql:27: Column 1 selects sink.getStringConstruction - // ql/src/Security/CWE-078/UnsafeShellCommandConstruction.ql:29: Column 7 selects sink.getCommandExecution - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(Sink).getLocation() + or + result = sink.(Sink).getStringConstruction().getLocation() + or + result = sink.(Sink).getCommandExecution().getLocation() } } diff --git a/python/ql/lib/semmle/python/security/dataflow/WeakSensitiveDataHashingQuery.qll b/python/ql/lib/semmle/python/security/dataflow/WeakSensitiveDataHashingQuery.qll index 9efa8320f97b..a219eac00b20 100644 --- a/python/ql/lib/semmle/python/security/dataflow/WeakSensitiveDataHashingQuery.qll +++ b/python/ql/lib/semmle/python/security/dataflow/WeakSensitiveDataHashingQuery.qll @@ -34,11 +34,7 @@ module NormalHashFunction { sensitiveDataExtraStepForCalls(node1, node2) } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/lib/semmle/python/security/dataflow/WeakSensitiveDataHashingQuery.qll:88: Flow call outside 'select' clause - none() - } + predicate observeDiffInformedIncrementalMode() { any() } } /** Global taint-tracking for detecting "use of a broken or weak cryptographic hashing algorithm on sensitive data" vulnerabilities. */ @@ -70,11 +66,7 @@ module ComputationallyExpensiveHashFunction { sensitiveDataExtraStepForCalls(node1, node2) } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/lib/semmle/python/security/dataflow/WeakSensitiveDataHashingQuery.qll:95: Flow call outside 'select' clause - none() - } + predicate observeDiffInformedIncrementalMode() { any() } } /** Global taint-tracking for detecting "use of a broken or weak cryptographic hashing algorithm on passwords" vulnerabilities. */ diff --git a/python/ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll b/python/ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll index 03f84b7903da..34649c0fb860 100644 --- a/python/ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll +++ b/python/ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll @@ -173,10 +173,7 @@ private module UntrustedDataToExternalApiConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { sink instanceof ExternalApiDataNode } predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll:181: Flow call outside 'select' clause - // ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll:184: Flow call outside 'select' clause - none() + none() // Not used for PR analysis } } diff --git a/python/ql/src/Security/CWE-327/FluentApiModel.qll b/python/ql/src/Security/CWE-327/FluentApiModel.qll index d2118493e0fe..8dd90a588217 100644 --- a/python/ql/src/Security/CWE-327/FluentApiModel.qll +++ b/python/ql/src/Security/CWE-327/FluentApiModel.qll @@ -112,9 +112,7 @@ module InsecureContextConfiguration implements DataFlow::StateConfigSig { } predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/Security/CWE-327/FluentApiModel.qll:130: Flow call outside 'select' clause - none() + none() // Too complicated, but might be possible after some refactoring. } } diff --git a/python/ql/src/experimental/semmle/python/libraries/SmtpLib.qll b/python/ql/src/experimental/semmle/python/libraries/SmtpLib.qll index 6712c9279f2e..de93bac0934a 100644 --- a/python/ql/src/experimental/semmle/python/libraries/SmtpLib.qll +++ b/python/ql/src/experimental/semmle/python/libraries/SmtpLib.qll @@ -40,9 +40,7 @@ module SmtpLib { } predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/experimental/semmle/python/libraries/SmtpLib.qll:91: Flow call outside 'select' clause - none() + none() // Used in library model } }