From bf74481f65a6efa4bb9ae39dd98ad9e4c27f5766 Mon Sep 17 00:00:00 2001 From: erik-krogh Date: Tue, 4 Oct 2022 13:41:50 +0200 Subject: [PATCH 1/4] add a link to the source in the alert-message for `rb/kernel-open` --- ruby/ql/src/queries/security/cwe-078/KernelOpen.ql | 4 ++-- ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql b/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql index f225128cf15..a292a28a70c 100644 --- a/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql +++ b/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql @@ -73,5 +73,5 @@ where sourceNode = source.getNode() and call.asExpr().getExpr().(MethodCall).getArgument(0) = sink.getNode().asExpr().getExpr() select sink.getNode(), source, sink, - "This call to " + call.(Replacement).getFrom() + - " depends on a user-provided value. Replace it with " + call.(Replacement).getTo() + "." + "This call to " + call.(Replacement).getFrom() + " depends on a . Replace it with " + + call.(Replacement).getTo() + ".", source.getNode(), "user-provided value" diff --git a/ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected b/ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected index 5659fceb1e7..996df218c0f 100644 --- a/ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected +++ b/ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected @@ -9,5 +9,5 @@ nodes | KernelOpen.rb:5:13:5:16 | file | semmle.label | file | subpaths #select -| KernelOpen.rb:4:10:4:13 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:4:10:4:13 | file | This call to Kernel.open depends on a user-provided value. Replace it with File.open. | -| KernelOpen.rb:5:13:5:16 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:5:13:5:16 | file | This call to IO.read depends on a user-provided value. Replace it with File.read. | +| KernelOpen.rb:4:10:4:13 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:4:10:4:13 | file | This call to Kernel.open depends on a . Replace it with File.open. | KernelOpen.rb:3:12:3:17 | call to params | user-provided value | +| KernelOpen.rb:5:13:5:16 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:5:13:5:16 | file | This call to IO.read depends on a . Replace it with File.read. | KernelOpen.rb:3:12:3:17 | call to params | user-provided value | From d370b2a51ed80fff458fc875314032a17f904a45 Mon Sep 17 00:00:00 2001 From: erik-krogh Date: Tue, 4 Oct 2022 13:49:50 +0200 Subject: [PATCH 2/4] simplify the where clause of `rb/kernel-open` --- ruby/ql/src/queries/security/cwe-078/KernelOpen.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql b/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql index a292a28a70c..fb0153bdd1e 100644 --- a/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql +++ b/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql @@ -71,7 +71,7 @@ from where config.hasFlowPath(source, sink) and sourceNode = source.getNode() and - call.asExpr().getExpr().(MethodCall).getArgument(0) = sink.getNode().asExpr().getExpr() + call.getArgument(0) = sink.getNode() select sink.getNode(), source, sink, "This call to " + call.(Replacement).getFrom() + " depends on a . Replace it with " + call.(Replacement).getTo() + ".", source.getNode(), "user-provided value" From 5ba7c13ecd927b50c3c4a4234ad3826dc37ea9bd Mon Sep 17 00:00:00 2001 From: Erik Krogh Kristensen Date: Tue, 4 Oct 2022 13:50:25 +0200 Subject: [PATCH 3/4] fix alert-message by adding the link Co-authored-by: Arthur Baars --- ruby/ql/src/queries/security/cwe-078/KernelOpen.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql b/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql index fb0153bdd1e..e2390944c1e 100644 --- a/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql +++ b/ruby/ql/src/queries/security/cwe-078/KernelOpen.ql @@ -73,5 +73,5 @@ where sourceNode = source.getNode() and call.getArgument(0) = sink.getNode() select sink.getNode(), source, sink, - "This call to " + call.(Replacement).getFrom() + " depends on a . Replace it with " + + "This call to " + call.(Replacement).getFrom() + " depends on a $@. Replace it with " + call.(Replacement).getTo() + ".", source.getNode(), "user-provided value" From dedbe6661906ddaad6273cdfa054991b70dd687b Mon Sep 17 00:00:00 2001 From: erik-krogh Date: Tue, 4 Oct 2022 14:16:07 +0200 Subject: [PATCH 4/4] update expected output --- ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected b/ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected index 996df218c0f..fc87de5c103 100644 --- a/ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected +++ b/ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected @@ -9,5 +9,5 @@ nodes | KernelOpen.rb:5:13:5:16 | file | semmle.label | file | subpaths #select -| KernelOpen.rb:4:10:4:13 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:4:10:4:13 | file | This call to Kernel.open depends on a . Replace it with File.open. | KernelOpen.rb:3:12:3:17 | call to params | user-provided value | -| KernelOpen.rb:5:13:5:16 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:5:13:5:16 | file | This call to IO.read depends on a . Replace it with File.read. | KernelOpen.rb:3:12:3:17 | call to params | user-provided value | +| KernelOpen.rb:4:10:4:13 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:4:10:4:13 | file | This call to Kernel.open depends on a $@. Replace it with File.open. | KernelOpen.rb:3:12:3:17 | call to params | user-provided value | +| KernelOpen.rb:5:13:5:16 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:5:13:5:16 | file | This call to IO.read depends on a $@. Replace it with File.read. | KernelOpen.rb:3:12:3:17 | call to params | user-provided value |