From 1c136e3cd0a0b2e76c7467124b2c66c72f15bf79 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 23 Jan 2025 10:33:58 +0100 Subject: [PATCH 1/2] Ruby: rerun patch query after bugfix --- .../codeql/ruby/security/ConditionalBypassQuery.qll | 6 +++++- .../codeql/ruby/security/InsecureDownloadQuery.qll | 6 +++++- .../ruby/security/UnsafeCodeConstructionQuery.qll | 6 +++++- .../ruby/security/UnsafeHtmlConstructionQuery.qll | 6 +++++- .../security/UnsafeShellCommandConstructionQuery.qll | 7 ++++++- .../ruby/security/WeakSensitiveDataHashingQuery.qll | 12 ++++++++++-- .../ruby/security/regexp/MissingFullAnchorQuery.qll | 7 ++++++- .../ruby/security/regexp/PolynomialReDoSQuery.qll | 7 ++++++- .../decompression-api/DecompressionApi.ql | 6 +++++- .../queries/security/cwe-732/WeakFilePermissions.ql | 6 +++++- 10 files changed, 58 insertions(+), 11 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll b/ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll index fee3990f09b8..6f0aa8a66f63 100644 --- a/ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll @@ -18,7 +18,11 @@ private module Config implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/src/experimental/cwe-807/ConditionalBypass.ql:78: Flow call outside 'select' clause + none() + } } /** diff --git a/ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll b/ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll index 641766acc707..38ac6a4e789a 100644 --- a/ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll @@ -21,7 +21,11 @@ private module InsecureDownloadConfig implements DataFlow::StateConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/src/queries/security/cwe-829/InsecureDownload.ql:20: Column 5 selects sink.getDownloadCall + none() + } } /** diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll index 24f86a2687a7..39709ec17d7c 100644 --- a/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll @@ -25,7 +25,11 @@ private module UnsafeCodeConstructionConfig implements DataFlow::ConfigSig { // override to require the path doesn't have unmatched return steps DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/src/queries/security/cwe-094/UnsafeCodeConstruction.ql:25: Column 7 selects sink.getCodeSink + none() + } } /** diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll index 75b73162b2a4..b25922f2f711 100644 --- a/ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll @@ -22,7 +22,11 @@ private module UnsafeHtmlConstructionConfig implements DataFlow::ConfigSig { // override to require the path doesn't have unmatched return steps DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/src/queries/security/cwe-079/UnsafeHtmlConstruction.ql:24: Column 7 selects sink.getXssSink + none() + } } /** diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll index 6041c38130e2..c367031cbad1 100644 --- a/ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll @@ -27,7 +27,12 @@ private module UnsafeShellCommandConstructionConfig implements DataFlow::ConfigS // override to require the path doesn't have unmatched return steps DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/src/queries/security/cwe-078/UnsafeShellCommandConstruction.ql:26: Column 1 selects sink.getStringConstruction + // ql/src/queries/security/cwe-078/UnsafeShellCommandConstruction.ql:28: Column 7 selects sink.getCommandExecution + none() + } } /** diff --git a/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll b/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll index 7ce8d213d2f5..af99e5c1e3ff 100644 --- a/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll @@ -29,7 +29,11 @@ module NormalHashFunction { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll:83: Flow call outside 'select' clause + none() + } } /** Global taint-tracking for detecting "use of a broken or weak cryptographic hashing algorithm on sensitive data" vulnerabilities. */ @@ -57,7 +61,11 @@ module ComputationallyExpensiveHashFunction { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll:90: Flow call outside 'select' clause + none() + } } /** Global taint-tracking for detecting "use of a broken or weak cryptographic hashing algorithm on passwords" vulnerabilities. */ diff --git a/ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll b/ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll index 6ff8843d4588..64f1ebf736ca 100644 --- a/ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll @@ -18,7 +18,12 @@ private module MissingFullAnchorConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/src/queries/security/cwe-020/MissingFullAnchor.ql:20: Column 7 selects sink.getCallNode + // ql/src/queries/security/cwe-020/MissingFullAnchor.ql:20: Column 9 selects sink.getRegex + none() + } } /** diff --git a/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll b/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll index e0b7d59bb1f8..61fcf39e865a 100644 --- a/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll @@ -19,7 +19,12 @@ private module PolynomialReDoSConfig implements DataFlow::ConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/src/queries/security/cwe-1333/PolynomialReDoS.ql:27: Column 1 selects sink.getHighlight + // ql/src/queries/security/cwe-1333/PolynomialReDoS.ql:29: Column 5 selects sink.getRegExp + none() + } } /** diff --git a/ruby/ql/src/experimental/decompression-api/DecompressionApi.ql b/ruby/ql/src/experimental/decompression-api/DecompressionApi.ql index aeb559417307..bd039611abf5 100644 --- a/ruby/ql/src/experimental/decompression-api/DecompressionApi.ql +++ b/ruby/ql/src/experimental/decompression-api/DecompressionApi.ql @@ -40,7 +40,11 @@ private module DecompressionApiConfig implements DataFlow::ConfigSig { // our Decompression APIs defined above will be the sinks we use for this query predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionApiUse } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/src/experimental/decompression-api/DecompressionApi.ql:54: Column 5 selects sink.getCall + none() + } } private module DecompressionApiFlow = TaintTracking::Global; diff --git a/ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql b/ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql index 478e758d844c..25a84acbf271 100644 --- a/ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql +++ b/ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql @@ -55,7 +55,11 @@ private module PermissivePermissionsConfig implements DataFlow::ConfigSig { exists(FileSystemPermissionModification mod | mod.getAPermissionNode() = sink) } - predicate observeDiffInformedIncrementalMode() { any() } + predicate observeDiffInformedIncrementalMode() { + // TODO(diff-informed): Manually verify if config can be diff-informed. + // ql/src/queries/security/cwe-732/WeakFilePermissions.ql:71: Column 5 does not select a source or sink originating from the flow call on line 69 + none() + } } private module PermissivePermissionsFlow = DataFlow::Global; From fcb8cac93019a0ecc53a28def26e883534d0992b Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 23 Jan 2025 11:43:00 +0100 Subject: [PATCH 2/2] Ruby: resolve inserted TODOs --- .../ruby/security/ConditionalBypassQuery.qll | 8 ++++---- .../ruby/security/InsecureDownloadQuery.qll | 10 ++++++---- .../security/UnsafeCodeConstructionQuery.qll | 10 ++++++---- .../security/UnsafeHtmlConstructionQuery.qll | 10 ++++++---- .../UnsafeShellCommandConstructionQuery.qll | 13 +++++++----- .../WeakSensitiveDataHashingQuery.qll | 12 ++--------- .../regexp/MissingFullAnchorQuery.qll | 13 +++++++----- .../security/regexp/PolynomialReDoSQuery.qll | 13 +++++++----- .../decompression-api/DecompressionApi.ql | 10 ++++++---- .../security/cwe-732/WeakFilePermissions.ql | 20 ++++++++++++------- 10 files changed, 67 insertions(+), 52 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll b/ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll index 6f0aa8a66f63..b6c711cc3c25 100644 --- a/ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/ConditionalBypassQuery.qll @@ -18,10 +18,10 @@ private module Config 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/experimental/cwe-807/ConditionalBypass.ql:78: Flow call outside 'select' clause - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.getLocation() or result = sink.(Sink).getAction().getLocation() } } diff --git a/ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll b/ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll index 38ac6a4e789a..c2d7437c169d 100644 --- a/ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/InsecureDownloadQuery.qll @@ -21,10 +21,12 @@ private module InsecureDownloadConfig implements DataFlow::StateConfigSig { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/queries/security/cwe-829/InsecureDownload.ql:20: Column 5 selects sink.getDownloadCall - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(Sink).getLocation() + or + result = sink.(Sink).getDownloadCall().getLocation() } } diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll index 39709ec17d7c..0237183bc745 100644 --- a/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/UnsafeCodeConstructionQuery.qll @@ -25,10 +25,12 @@ private module UnsafeCodeConstructionConfig 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/queries/security/cwe-094/UnsafeCodeConstruction.ql:25: Column 7 selects sink.getCodeSink - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(Sink).getLocation() + or + result = sink.(Sink).getCodeSink().getLocation() } } diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll index b25922f2f711..e1a6e4f21dc2 100644 --- a/ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/UnsafeHtmlConstructionQuery.qll @@ -22,10 +22,12 @@ private module UnsafeHtmlConstructionConfig 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/queries/security/cwe-079/UnsafeHtmlConstruction.ql:24: Column 7 selects sink.getXssSink - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(Sink).getLocation() + or + result = sink.(Sink).getXssSink().getLocation() } } diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll index c367031cbad1..c46ebd6e9d59 100644 --- a/ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/UnsafeShellCommandConstructionQuery.qll @@ -27,11 +27,14 @@ private module UnsafeShellCommandConstructionConfig implements DataFlow::ConfigS // 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/queries/security/cwe-078/UnsafeShellCommandConstruction.ql:26: Column 1 selects sink.getStringConstruction - // ql/src/queries/security/cwe-078/UnsafeShellCommandConstruction.ql:28: 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/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll b/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll index af99e5c1e3ff..7ce8d213d2f5 100644 --- a/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll @@ -29,11 +29,7 @@ module NormalHashFunction { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll:83: 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. */ @@ -61,11 +57,7 @@ module ComputationallyExpensiveHashFunction { predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/lib/codeql/ruby/security/WeakSensitiveDataHashingQuery.qll:90: 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/ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll b/ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll index 64f1ebf736ca..d511c6f3fbf8 100644 --- a/ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/regexp/MissingFullAnchorQuery.qll @@ -18,11 +18,14 @@ private module MissingFullAnchorConfig 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/queries/security/cwe-020/MissingFullAnchor.ql:20: Column 7 selects sink.getCallNode - // ql/src/queries/security/cwe-020/MissingFullAnchor.ql:20: Column 9 selects sink.getRegex - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(Sink).getLocation() + or + result = sink.(Sink).getCallNode().getLocation() + or + result = sink.(Sink).getRegex().getLocation() } } diff --git a/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll b/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll index 61fcf39e865a..562b5dad37bf 100644 --- a/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll +++ b/ruby/ql/lib/codeql/ruby/security/regexp/PolynomialReDoSQuery.qll @@ -19,11 +19,14 @@ private module PolynomialReDoSConfig 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/queries/security/cwe-1333/PolynomialReDoS.ql:27: Column 1 selects sink.getHighlight - // ql/src/queries/security/cwe-1333/PolynomialReDoS.ql:29: Column 5 selects sink.getRegExp - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(Sink).getLocation() + or + result = sink.(Sink).getHighlight().getLocation() + or + result = sink.(Sink).getRegExp().getLocation() } } diff --git a/ruby/ql/src/experimental/decompression-api/DecompressionApi.ql b/ruby/ql/src/experimental/decompression-api/DecompressionApi.ql index bd039611abf5..956d952789ce 100644 --- a/ruby/ql/src/experimental/decompression-api/DecompressionApi.ql +++ b/ruby/ql/src/experimental/decompression-api/DecompressionApi.ql @@ -40,10 +40,12 @@ private module DecompressionApiConfig implements DataFlow::ConfigSig { // our Decompression APIs defined above will be the sinks we use for this query predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionApiUse } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/experimental/decompression-api/DecompressionApi.ql:54: Column 5 selects sink.getCall - none() + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + result = sink.(DecompressionApiUse).getLocation() + or + result = sink.(DecompressionApiUse).getCall().getLocation() } } diff --git a/ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql b/ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql index 25a84acbf271..dbc5db91d998 100644 --- a/ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql +++ b/ruby/ql/src/queries/security/cwe-732/WeakFilePermissions.ql @@ -51,14 +51,19 @@ private module PermissivePermissionsConfig implements DataFlow::ConfigSig { source.asExpr().getExpr() instanceof PermissivePermissionsExpr } - predicate isSink(DataFlow::Node sink) { - exists(FileSystemPermissionModification mod | mod.getAPermissionNode() = sink) + additional predicate sinkDef(DataFlow::Node sink, FileSystemPermissionModification mod) { + mod.getAPermissionNode() = sink } - predicate observeDiffInformedIncrementalMode() { - // TODO(diff-informed): Manually verify if config can be diff-informed. - // ql/src/queries/security/cwe-732/WeakFilePermissions.ql:71: Column 5 does not select a source or sink originating from the flow call on line 69 - none() + predicate isSink(DataFlow::Node sink) { sinkDef(sink, _) } + + predicate observeDiffInformedIncrementalMode() { any() } + + Location getASelectedSinkLocation(DataFlow::Node sink) { + exists(FileSystemPermissionModification mod | + sinkDef(sink, mod) and + result = mod.getLocation() + ) } } @@ -70,7 +75,8 @@ from PermissivePermissionsFlow::PathNode source, PermissivePermissionsFlow::PathNode sink, FileSystemPermissionModification mod where - PermissivePermissionsFlow::flowPath(source, sink) and mod.getAPermissionNode() = sink.getNode() + PermissivePermissionsFlow::flowPath(source, sink) and + PermissivePermissionsConfig::sinkDef(sink.getNode(), mod) select source.getNode(), source, sink, "This overly permissive mask used in $@ allows read or write access to others.", mod, mod.toString()