Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,12 @@
<li> <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-9182">CVE-2016-9182</a> </li>
<li> <a href="http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-1000210">CVE-2018-1000210</a> </li>
</ul>
<p>It is dangerous to let external sources either:</p>
<ul>
<li> execute unknown code in your process. Such <a href="https://www.owasp.org/index.php/Code_Injection">Injected Code</a> can either run on the
server or in the client (exemple: XSS attack). </li>
<li> select the code which will be executed via reflection. </li>
</ul>
<p>It is dangerous to execute unknown code in your process. Such <a href="https://www.owasp.org/index.php/Code_Injection">Injected Code</a> can either
run on the server or in the client (exemple: XSS attack).</p>
<p>This rule marks for review each occurence of such dynamic code execution. The goal is to guide security code reviews.</p>
<h2>Ask Yourself Whether</h2>
<ul>
<li> the executed code may come from a untrusted source and hasn't been sanitized. </li>
<li> the code to run is dynamically chosen via reflection, and an untrusted source can use it to choose which code to run. For example a class could
be retrieved by its name and this name comes from a user input. </li>
</ul>
<p>You are at risk if you answered yes to any of these questions.</p>
<h2>Recommended Secure Coding Practices</h2>
Expand All @@ -27,40 +21,33 @@ <h2>Recommended Secure Coding Practices</h2>
href="https://www.w3schools.com/tags/att_iframe_sandbox.asp">iframes</a> and <a href="https://en.wikipedia.org/wiki/Same-origin_policy">same-origin
policy</a> for javascript in a web browser).</p>
<p>Do not try to create a blacklist of dangerous code. It is impossible to cover all attacks that way.</p>
<p>As for the use of reflection, it should be strictly controlled as it can lead to many vulnerabilities. Never let an untrusted source decide what
code to run. If you have to do it anyway, create a list of allowed code and choose among this list.</p>
<h2>Exceptions</h2>
<p>Calling reflection methods with a hard-coded type name, method name or field name will not raise an issue.</p>
<h2>See</h2>
<ul>
<li> <a href="http://cwe.mitre.org/data/definitions/95.html">MITRE CWE-95</a> - Improper Neutralization of Directives in Dynamically Evaluated Code
('Eval Injection') </li>
<li> <a href="http://cwe.mitre.org/data/definitions/470.html">MITRE CWE-470</a> - Use of Externally-Controlled Input to Select Classes or Code
('Unsafe Reflection') </li>
<li> OWASP Top 10 2017 Category A1 - Injection </li>
<li> OWASP Top 10 2017 Category A7 - Cross-Site Scripting (XSS) </li>
</ul>
<h2>Questionable Code Example</h2>
<pre>
import os

value = input()
formatted_command = 'os.system("%s")' % value
compiled_command = compile(hardcoded_command, '&lt;string&gt;', 'eval')
compiled_code = compile(hardcoded_command, '&lt;string&gt;', 'exec')
command = 'os.system("%s")' % value

def evaluate(command, file, mode):
eval(command) # Questionable. Dynamic command
eval(compile(command, file, mode)) # Questionable. Dynamic command
eval(command) # Questionable.

eval(formatted_command) # Questionable. Dynamic code
eval(compiled_command) # Questionable even though it is hardcoded, but only because it is not worth detecting this corner case.
eval(command) # Questionable. Dynamic code

def execute(code, file, mode):
exec(code) # Questionable. Dynamic code
exec(compile(code, file, mode)) # Questionable. Dynamic command
exec(code) # Questionable.
exec(compile(code, file, mode)) # Questionable.

exec(formatted_command) # Questionable. Dynamic code
exec(compiled_code) # Questionable even though it is hardcoded, but only because it is not worth detecting this corner case.
exec(command) # Questionable.
</pre>
<h2>Exceptions</h2>
<p>None</p>
<h2>See</h2>
<ul>
<li> <a href="http://cwe.mitre.org/data/definitions/95.html">MITRE CWE-95</a> - Improper Neutralization of Directives in Dynamically Evaluated Code
('Eval Injection') </li>
<li> <a href="http://cwe.mitre.org/data/definitions/470.html">MITRE CWE-470</a> - Use of Externally-Controlled Input to Select Classes or Code
('Unsafe Reflection') </li>
<li> OWASP Top 10 2017 Category A1 - Injection </li>
<li> OWASP Top 10 2017 Category A7 - Cross-Site Scripting (XSS) </li>
</ul>

Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
execute SQL commands should sanitize any externally-provided values used in those commands. Failure to do so could allow an attacker to include input
that changes the query so that unintended commands are executed, or sensitive data is exposed. Instead of trying to sanitize data by hand, SQL binding
mechanisms should be used; they can be relied on to automatically perform a full sanitization.</p>
<p>This rule flags the execution of SQL queries via Django methods which might be susceptible to SQL injection. Any SQL query built by concatenating
or formatting Strings is considered susceptible. The goal is to guide security code reviews.</p>
<p>This rule flags the execution of SQL queries via Django methods which are not recommended by Django documentation as their use can result in an SQL
injection. The goal is to guide security code reviews.</p>
<h2>Recommended Secure Coding Practices</h2>
<ul>
<li> Avoid building queries manually using concatenation or formatting. If you do it anyway, do not include user input in this building process.
Expand All @@ -34,32 +34,18 @@ <h2>Questionable Code Example</h2>
class MyUser(models.Model):
name = models.CharField(max_length=200)

hardcoded_request = 'SELECT * FROM mytable WHERE name = "test"'
formatted_request = 'SELECT * FROM mytable WHERE name = "%s"' % value

def query_my_user(request, params):
MyUser.objects.raw(request) # Questionable
MyUser.objects.raw(formatted_request) # Questionable
MyUser.objects.raw(hardcoded_request) # Ok

# Parametrized queries
MyUser.objects.raw(request, params) # Questionable.
MyUser.objects.raw(formatted_request, params) # Questionable.

# Note: Adding quotes around %s can result in SQL injections
MyUser.objects.raw(hardcoded_request, params) # Questionable. Review in case the request has quoted parameters.

with connection.cursor() as cursor:
cursor.execute(request) # Questionable
cursor.execute(formatted_request) # Questionable
cursor.execute(hardcoded_request, params) # Questionable, See "Note"
cursor.execute(hardcoded_request) # Ok

with connections['my_db'].cursor() as cursor:
cursor.execute(request) # Questionable
cursor.execute(formatted_request) # Questionable
cursor.execute(hardcoded_request, params) # Questionable, See "Note"
cursor.execute(hardcoded_request) # Ok

# https://docs.djangoproject.com/en/2.1/ref/models/expressions/#raw-sql-expressions

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,4 @@ <h2>Questionable Code Example</h2>
DEBUG = True # Questionable
DEBUG_PROPAGATE_EXCEPTIONS = True # Questionable
</pre>
<p>Flask</p>
<pre>
from flask import Flask

app = Flask(__name__)
app.config['TESTING'] = True # Questionable
app.config['DEBUG'] = True # Questionable
app.config['PROPAGATE_EXCEPTIONS'] = True # Questionable
app.config['PRESERVE_CONTEXT_ON_EXCEPTION'] = True # Questionable

app.testing = True # Questionable
app.debug = True # Questionable

app.config.update(
TESTING=True, # Questionable
DEBUG=True, # Questionable
PROPAGATE_EXCEPTIONS=True, # Questionable
PRESERVE_CONTEXT_ON_EXCEPTION=True # Questionable
)
</pre>

Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,6 @@ <h2>Recommended Secure Coding Practices</h2>
<p>Restrict security-sensitive actions, such as file upload, to authenticated users.</p>
<p>Be careful when errors are returned to the client, as they can provide sensitive information. Use 404 (Not Found) instead of 403 (Forbidden) when
the existence of a resource is sensitive.</p>
<h2>Questionable Code Example</h2>
<p>Django endpoint declaration</p>
<pre>
from django.urls import path, re_path

def declare_views(views):
return [
path('', views['index']), # Questionable
re_path(r'^about/[0-9]*/$', views['about']), # Questionable
]
</pre>
<h2>See</h2>
<ul>
<li> <a href="http://cwe.mitre.org/data/definitions/20.html">MITRE, CWE-20</a> - Improper Input Validation </li>
Expand All @@ -68,4 +57,15 @@ <h2>See</h2>
<li> <a href="https://www.sans.org/top25-software-errors/#cat2">SANS Top 25</a> - Risky Resource Management </li>
<li> <a href="https://www.sans.org/top25-software-errors/#cat3">SANS Top 25</a> - Porous Defenses </li>
</ul>
<h2>Questionable Code Example</h2>
<p>Django endpoint declaration</p>
<pre>
from django.urls import path, re_path

def declare_views(views):
return [
path('', views['index']), # Questionable
re_path(r'^about/[0-9]*/$', views['about']), # Questionable
]
</pre>

Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ <h2>Recommended Secure Coding Practices</h2>
<li> refuse to run the command if the process has too many privileges. For example: forbid running the code as "root". </li>
</ul>
<p> </p>
<h2>See</h2>
<ul>
<li> <a href="http://cwe.mitre.org/data/definitions/78">MITRE, CWE-78</a> - Improper Neutralization of Special Elements used in an OS Command </li>
<li> OWASP Top 10 2017 Category A1 - Injection </li>
<li> <a href="https://www.sans.org/top25-software-errors/#cat1">SANS Top 25</a> - Insecure Interaction Between Components </li>
</ul>
<h2>Questionable Code Example</h2>
<p>Python 3</p>
<pre>
Expand Down Expand Up @@ -102,10 +108,4 @@ <h2>Questionable Code Example</h2>
(child_stdout, _, _) = popen2.popen3(cmd) # Questionable
(child_stdout, _) = popen2.popen4(cmd) # Questionable
</pre>
<h2>See</h2>
<ul>
<li> <a href="http://cwe.mitre.org/data/definitions/78">MITRE, CWE-78</a> - Improper Neutralization of Special Elements used in an OS Command </li>
<li> OWASP Top 10 2017 Category A1 - Injection </li>
<li> <a href="https://www.sans.org/top25-software-errors/#cat1">SANS Top 25</a> - Insecure Interaction Between Components </li>
</ul>

Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,6 @@ <h2>Recommended Secure Coding Practices</h2>
make complex regular expressions as they are difficult to understand and test. Note that some regular expression engines will match only part of the
input if no anchors are used. In PHP for example <code>preg_match("/[A-Za-z0-9]+/", $text)</code> will accept any string containing at least one
alphanumeric character because it has no anchors.</p>
<h2>Exceptions</h2>
<p>Regardless of the string being matched, a hardcoded regular expression pattern is not vulnerable to ReDoS attacks if it consists only of one
character or only alphanumeric characters. No issue will be raised for these cases.</p>
<h2>See</h2>
<ul>
<li> <a href="https://www.owasp.org/index.php/Regular_expression_Denial_of_Service_-_ReDoS">MITRA, CWE-624</a> - Executable Regular Expression Error
</li>
<li> <a href="https://cwe.mitre.org/data/definitions/185.html">MITRA, CWE-185</a> - Incorrect Regular Expression </li>
<li> OWASP Regular expression Denial of Service - ReDoS </li>
<li> OWASP Top 10 2017 Category A1 - Injection </li>
<li> <a href="https://www.sans.org/top25-software-errors/#cat3">SANS Top 25</a> - Porous Defenses </li>
</ul>
<h2>Questionable Code Example</h2>
<p>Django</p>
<pre>
Expand Down Expand Up @@ -110,7 +98,6 @@ <h2>Questionable Code Example</h2>

input = 'input string'
replacement = 'replacement'
simple_hardcoded_pattern = r'pattern'

regex.subf # Questionable
regex.subfn # Questionable
Expand All @@ -125,10 +112,6 @@ <h2>Questionable Code Example</h2>
regex.subfn(pattern, replacement, input) # Questionable
regex.splititer(pattern, input) # Questionable

regex.subf(hardcoded_pattern, replacement, input) # Ok
regex.subfn(hardcoded_pattern, replacement, input) # Ok
regex.splititer(hardcoded_pattern, input) # Ok

regex.compile # Questionable
regex.match # Questionable
regex.search # Questionable
Expand Down Expand Up @@ -160,14 +143,16 @@ <h2>Questionable Code Example</h2>
regex.sub(pattern, replacement, input) # Questionable
regex.subn(pattern, replacement, input) # Questionable

regex.compile(simple_hardcoded_pattern) # Ok
regex.match(simple_hardcoded_pattern, input) # Ok
regex.search(simple_hardcoded_pattern, input) # Ok
regex.fullmatch(simple_hardcoded_pattern, input) # Ok
regex.split(simple_hardcoded_pattern, input) # Ok
regex.findall(simple_hardcoded_pattern, input) # Ok
regex.finditer(simple_hardcoded_pattern, input) # Ok
regex.sub(simple_hardcoded_pattern, replacement, input) # Ok
regex.subn(simple_hardcoded_pattern, replacement, input) # Ok
</pre>
<h2>Exceptions</h2>
<p>None</p>
<h2>See</h2>
<ul>
<li> <a href="https://www.owasp.org/index.php/Regular_expression_Denial_of_Service_-_ReDoS">MITRA, CWE-624</a> - Executable Regular Expression Error
</li>
<li> <a href="https://cwe.mitre.org/data/definitions/185.html">MITRA, CWE-185</a> - Incorrect Regular Expression </li>
<li> OWASP Regular expression Denial of Service - ReDoS </li>
<li> OWASP Top 10 2017 Category A1 - Injection </li>
<li> <a href="https://www.sans.org/top25-software-errors/#cat3">SANS Top 25</a> - Porous Defenses </li>
</ul>

Loading