From 3811eae67946eba9814c4945fd77e554890dc76c Mon Sep 17 00:00:00 2001
From: erik-krogh <erik-krogh@github.com>
Date: Mon, 2 Jan 2023 11:26:58 +0100
Subject: [PATCH] simplify the qhelp for unsafe-code-construction The `send()`
 example is not flagged by any current query, so it was weird talking about it
 as "vulnerable".

---
 .../cwe-094/UnsafeCodeConstruction.qhelp      | 19 ++++++++++---------
 .../examples/UnsafeCodeConstruction3Safe.rb   |  1 -
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/ruby/ql/src/queries/security/cwe-094/UnsafeCodeConstruction.qhelp b/ruby/ql/src/queries/security/cwe-094/UnsafeCodeConstruction.qhelp
index 63f566aaa57..6936aee149d 100644
--- a/ruby/ql/src/queries/security/cwe-094/UnsafeCodeConstruction.qhelp
+++ b/ruby/ql/src/queries/security/cwe-094/UnsafeCodeConstruction.qhelp
@@ -54,11 +54,11 @@ a class that has a getter method with a custom name.
 <p>
 The example dynamically constructs a string which is then executed using <code>module_eval</code>.  
 This code will break if the specified name is not a valid Ruby identifier, and 
-if the value is controlled by an attacker, then this could lead to code injection.
+if the value is controlled by an attacker, then this could lead to code-injection.
 </p>
 
 <p>
-A more robust implementation, that is also immune to code injection, 
+A more robust implementation, that is also immune to code-injection, 
 can be made by using <code>module_eval</code> with a block and using <code>define_method</code>
 to define the getter method.
 </p>
@@ -80,13 +80,14 @@ and methods.
 A safer approach is to use <code>class_variable_set</code> and
 <code>class_variable_get</code> along with <code>define_method</code>. String
 interpolation is still used to construct the class variable name, but this is
-safe because <code>class_variable_set</code> is not susceptible to code injection.
-To construct a dynamic method call we use <code>send</code>, which is ulnerable
-to code injection: if an attacker can control the first argument, they can call
-any method on the receiver. However this is less powerful than being able to run
-arbitrary Ruby code, so it is an improvement in security. We also document to
-callers that they should not pass arbitrary user data to the <code>name</code>
-parameter.
+safe because <code>class_variable_set</code> is not susceptible to code-injection.
+</p>
+
+<p>
+<code>send</code> is used to dynamically call the method specified by <code>name</code>.  
+This is a more robust alternative than the previous example, because it does not allow
+arbitrary code to be executed, but it does still allow for any method to be called
+on the target object. 
 </p>
 
 <sample src="examples/UnsafeCodeConstruction3Safe.rb" />
diff --git a/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3Safe.rb b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3Safe.rb
index 65608f8aa20..ac32224ce72 100644
--- a/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3Safe.rb
+++ b/ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3Safe.rb
@@ -1,5 +1,4 @@
 module Invoker
-  # Do not pass arbitrary user input to +name+.
   def attach(klass, name, target)
     var = :"@@#{name}"
     klass.class_variable_set(var, target)