Skip to content

Commit 3811eae

Browse files
committed
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".
1 parent 3815a5a commit 3811eae

File tree

2 files changed

+10
-10
lines changed

2 files changed

+10
-10
lines changed

ruby/ql/src/queries/security/cwe-094/UnsafeCodeConstruction.qhelp

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,11 @@ a class that has a getter method with a custom name.
5454
<p>
5555
The example dynamically constructs a string which is then executed using <code>module_eval</code>.
5656
This code will break if the specified name is not a valid Ruby identifier, and
57-
if the value is controlled by an attacker, then this could lead to code injection.
57+
if the value is controlled by an attacker, then this could lead to code-injection.
5858
</p>
5959

6060
<p>
61-
A more robust implementation, that is also immune to code injection,
61+
A more robust implementation, that is also immune to code-injection,
6262
can be made by using <code>module_eval</code> with a block and using <code>define_method</code>
6363
to define the getter method.
6464
</p>
@@ -80,13 +80,14 @@ and methods.
8080
A safer approach is to use <code>class_variable_set</code> and
8181
<code>class_variable_get</code> along with <code>define_method</code>. String
8282
interpolation is still used to construct the class variable name, but this is
83-
safe because <code>class_variable_set</code> is not susceptible to code injection.
84-
To construct a dynamic method call we use <code>send</code>, which is ulnerable
85-
to code injection: if an attacker can control the first argument, they can call
86-
any method on the receiver. However this is less powerful than being able to run
87-
arbitrary Ruby code, so it is an improvement in security. We also document to
88-
callers that they should not pass arbitrary user data to the <code>name</code>
89-
parameter.
83+
safe because <code>class_variable_set</code> is not susceptible to code-injection.
84+
</p>
85+
86+
<p>
87+
<code>send</code> is used to dynamically call the method specified by <code>name</code>.
88+
This is a more robust alternative than the previous example, because it does not allow
89+
arbitrary code to be executed, but it does still allow for any method to be called
90+
on the target object.
9091
</p>
9192

9293
<sample src="examples/UnsafeCodeConstruction3Safe.rb" />

ruby/ql/src/queries/security/cwe-094/examples/UnsafeCodeConstruction3Safe.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
module Invoker
2-
# Do not pass arbitrary user input to +name+.
32
def attach(klass, name, target)
43
var = :"@@#{name}"
54
klass.class_variable_set(var, target)

0 commit comments

Comments
 (0)