Just coding for fun#58
Conversation
| # HIGH: Hardcoded credentials | ||
| DATABASE_PASSWORD = "SuperSecret123!" | ||
| API_SECRET_KEY = "sk-prod-1234567890abcdef" | ||
| AWS_ACCESS_KEY = "AKIAIOSFODNN7EXAMPLE" |
There was a problem hiding this comment.
|
|
|
||
| // HIGH: SQL Injection in search | ||
| function searchProducts(searchTerm) { | ||
| const query = "SELECT * FROM products WHERE name LIKE '%" + searchTerm + "%'"; |
There was a problem hiding this comment.
🔴 High: Code Vulnerability
potential SQL injection (...read more)
Check for variable declarations in a SQL statement where there is potential for SQL injections.
| DATABASE_PASSWORD = "WHoGoesThere!" | ||
| API_SECRET_KEY = "sk-prod-1234567890abcdef" | ||
| AWS_ACCESS_KEY = "AKIAIOSFODNN7EXAMPLE" | ||
| AWS_SECRET_KEY = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" |
There was a problem hiding this comment.
| DATABASE_PASSWORD = "WHoGoesThere!" | ||
| API_SECRET_KEY = "sk-prod-1234567890abcdef" | ||
| AWS_ACCESS_KEY = "AKIAIOSFODNN7EXAMPLE" | ||
| AWS_SECRET_KEY = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" |
There was a problem hiding this comment.
|
❌ Code Quality 🛑 Potential blockersCaution 🛠️ 41 Code quality issues detectedNo new code quality violations gate threshold exceeded. High: python-best-practices/nested-blocks (Fix with Cursor)Do not have too many nested blocks View ruleapp/main.py:130-145 Medium: python-best-practices/avoid-string-concat (Fix with Cursor)avoid string concatenation View ruleapp/main.py:189 Medium: javascript-best-practices/no-console (Fix with Cursor)Avoid leaving console debug statements View ruleapp/cart.js:245
|
| if user: | ||
| if user['active']: | ||
| if discount > 0: | ||
| if shipping: | ||
| if tax: | ||
| x = 1 | ||
| else: | ||
| x = 2 | ||
| else: | ||
| x = 3 | ||
| else: | ||
| x = 4 | ||
| else: | ||
| x = 5 | ||
| else: | ||
| x = 6 |
There was a problem hiding this comment.
🔴 High: Code Quality Violation
too many nesting levels (...read more)
Avoid to nest too many loops together. Having too many loops make your code harder to understand.
Prefer to organize your code in functions and unit of code you can clearly understand.
Learn More
|
|
||
| # HIGH: Command Injection vulnerability | ||
| def generate_report(report_name): | ||
| os.system("echo 'Report' > /tmp/" + report_name + ".txt") |
There was a problem hiding this comment.
⚫ Critical: Code Vulnerability
| os.system("echo 'Report' > /tmp/" + report_name + ".txt") | |
| os.system(shlex.quote("echo 'Report' > /tmp/" + report_name + ".txt")) |
unsanitized process call. Use shlex.quote() to sanitize the call (...read more)
Detect unsafe shell execution with the os module. We should ensure the command is safe before execution. Use shlex to sanitize user inputs.
Learn More
| def get_user_by_id(user_id): | ||
| conn = sqlite3.connect('shopist.db') | ||
| cursor = conn.cursor() | ||
| query = "SELECT * FROM users WHERE id = " + user_id |
There was a problem hiding this comment.
⚫ Critical: Code Vulnerability
Potential SQL injection (...read more)
In Python, SQL injection is a code injection technique used to attack data-driven applications. This vulnerability occurs when an attacker can insert a SQL query through the input data from the client to the application. This is aimed at preventing SQL injection vulnerability.
Preventing SQL injections is critical because they can lead to the exposure of confidential information, loss of data integrity, and other serious database-related issues. If an attacker successfully performs a SQL injection, they might gain unauthorized access to sensitive data, including passwords, credit card numbers, or personal user information.
How to remediate
To avoid SQL injections, use parameterized queries or prepared statements instead of building SQL commands using string format or concatenation.
For example, instead of writing a query as:
cursor.execute("SELECT * FROM table WHERE column = '%s'" % value)You can use
cursor.execute("SELECT * FROM table WHERE column = ?", (value,))This way, the value is automatically escaped properly by the database driver, thus preventing SQL injection. Also, always validate and sanitize user inputs to ensure they do not contain SQL code.
|
|
||
| # HIGH: Command injection with subprocess | ||
| def backup_database(filename): | ||
| subprocess.call("pg_dump shopist > " + filename, shell=True) |
There was a problem hiding this comment.
⚫ Critical: Code Vulnerability
Command injection vulnerability: user input concatenated into shell command. (...read more)
This rule identifies potential OS Command Injection vulnerabilities in Python code. These vulnerabilities occur when an application constructs shell commands by incorporating user-supplied or otherwise untrusted data directly into command strings without proper sanitization. An attacker can exploit this by crafting input that includes malicious shell commands, which are then executed on the server with the privileges of the running application. This can lead to unauthorized server access, data breaches, arbitrary code execution, or denial of service.
The rule specifically looks for:
- Calls to
subprocessmodule functions (likerun,call,Popen, etc.) where the command argument is built using string formatting (f-strings,.format(),%operator) or concatenation with potentially unsanitized variables, especially whenshell=Truemight be implied or used. - Assignments to variables (often named
cmd,command,args, etc.) where the string value is constructed using these unsafe methods, and these variables are likely intended for command execution.
How to Remediate
To prevent OS Command Injection vulnerabilities when constructing and executing shell commands in Python:
-
Prefer List Arguments with
shell=False: When usingsubprocessfunctions (e.g.,subprocess.run(),subprocess.call()), pass the command and its arguments as a list of strings. Ensureshell=Falseis used (this is the default forsubprocess.run()). This method treats each item in the list as a literal argument and does not invoke a shell to interpret the command string, significantly reducing the risk of injection.import subprocess user_filename = "some_file.txt" # Example of an argument, potentially from user input # Compliant: command and arguments as a list, shell=False (default) try: result = subprocess.run(["cat", user_filename], capture_output=True, text=True, check=True) print(result.stdout) except subprocess.CalledProcessError as e: print(f"Error executing command: {e}") except FileNotFoundError: print("Error: Command not found.")
-
Use
shlex.quote()for Shell Execution: If you absolutely must useshell=Trueor need to construct a single command string that will be interpreted by the shell, sanitize any dynamic parts of the command string (especially those derived from external input) usingshlex.quote(). This function escapes characters that have special meaning to the shell, making the input safe.import subprocess import shlex user_input = "some value; malicious_command" # Example of user input safe_argument = shlex.quote(user_input) # Compliant: user input is quoted before being included in a command string for shell=True try: result = subprocess.run(f"echo {safe_argument}", shell=True, capture_output=True, text=True, check=True) print(result.stdout) except subprocess.CalledProcessError as e: print(f"Error executing command: {e}")
-
Avoid Direct String Concatenation/Formatting for Commands: Do not build command strings by directly concatenating or formatting them with untrusted input if the command will be processed by a shell.
# Non-compliant example: # user_directory = request.args.get("dir") # command = "ls -l " + user_directory # subprocess.run(command, shell=True) # Vulnerable # Non-compliant example: # search_term = request.args.get("term") # command = f"grep '{search_term}' /var/log/app.log" # subprocess.run(command, shell=True) # Vulnerable
Instead, use the list-based approach (Remediation Add terraform files to new demo repo #1) or
shlex.quote()(Remediation DataDog Code Security Demo #2). -
Validate and Sanitize All External Input: Beyond specific command construction, rigorously validate and sanitize any external input that influences program behavior, including what commands are run or what arguments are passed. Use allowlists for permissible inputs where possible.
-
Principle of Least Privilege: Run your application with the minimum privileges necessary. This will not prevent the injection itself but can limit the potential damage if such a vulnerability is exploited.
|
|
||
| // HIGH: SQL Injection vulnerability | ||
| function getUserById(userId) { | ||
| const query = "SELECT * FROM users WHERE id = " + userId; |
There was a problem hiding this comment.
⚫ Critical: Code Vulnerability
potential SQL injection (...read more)
Check for variable declarations in a SQL statement where there is potential for SQL injections.
Learn More
| if order['items']: | ||
| if len(order['items']) > 0: | ||
| if user: | ||
| if user['active']: |
There was a problem hiding this comment.
🟠 Medium: Code Quality Violation
Found too many nested ifs within this condition (...read more)
Too many nested loops make the code hard to read and understand. Simplify your code by removing nesting levels and separate code in small units.
| unused_var = "never used" | ||
|
|
||
| if order: | ||
| if order['items']: |
There was a problem hiding this comment.
🟠 Medium: Code Quality Violation
Found too many nested ifs within this condition (...read more)
Too many nested loops make the code hard to read and understand. Simplify your code by removing nesting levels and separate code in small units.
|
|
||
| # MEDIUM: Weak hashing (SHA1) | ||
| def generate_token(data): | ||
| return hashlib.sha1(data.encode()).hexdigest() |
There was a problem hiding this comment.
🔴 High: Code Vulnerability
| return hashlib.sha1(data.encode()).hexdigest() | |
| return hashlib.sha256(data.encode()).hexdigest() |
Use of insecure hashing method sha1 (...read more)
Do not use a broken or risky cryptographic algorithm. This exposes you to unwanted attacks.
It checks the following modules
Learn More
| const DEBUG = true; | ||
| function logDebug(message, data) { | ||
| if (DEBUG) { | ||
| console.log('DEBUG:', message, JSON.stringify(data)); |
There was a problem hiding this comment.
🟠 Medium: Code Quality Violation
Unexpected console statement. (...read more)
Debugging with console (such as console.log or console.info) is not considered a bad practice, but these statements can be accidentally left in production code, leading to unnecessary log pollution. It is important to remove or replace these debugging statements to maintain clean and secure production builds.
| } else if (code == "SAVE50") { | ||
| discount = 50; | ||
| } | ||
| console.log("Discount applied: " + discount); |
There was a problem hiding this comment.
🟠 Medium: Code Quality Violation
Unexpected console statement. (...read more)
Debugging with console (such as console.log or console.info) is not considered a bad practice, but these statements can be accidentally left in production code, leading to unnecessary log pollution. It is important to remove or replace these debugging statements to maintain clean and secure production builds.
|
|
||
| # MEDIUM: Weak hashing (SHA1) | ||
| def generate_token(data): | ||
| return hashlib.sha1(data.encode()).hexdigest() |
There was a problem hiding this comment.
🔴 High: Code Vulnerability
| return hashlib.sha1(data.encode()).hexdigest() | |
| return hashlib.sha256(data.encode()).hexdigest() |
Use of insecure hashing method sha1 (...read more)
Do not use a broken or risky cryptographic algorithm. This exposes you to unwanted attacks.
It checks the following modules
Learn More
|
|
||
| # MEDIUM: Weak hashing algorithm (MD5) | ||
| def hash_password(password): | ||
| return hashlib.md5(password.encode()).hexdigest() |
There was a problem hiding this comment.
🔴 High: Code Vulnerability
| return hashlib.md5(password.encode()).hexdigest() | |
| return hashlib.sha256(password.encode()).hexdigest() |
Use of insecure hashing method md5 (...read more)
Do not use a broken or risky cryptographic algorithm. This exposes you to unwanted attacks.
It checks the following modules
Learn More
|
|
||
| // MEDIUM: Weak cryptographic algorithm (MD5) | ||
| function hashPassword(password) { | ||
| return crypto.createHash('md5').update(password).digest('hex'); |
There was a problem hiding this comment.
🔴 High: Code Vulnerability
insecure hash algorithm md5 (...read more)
Do not use weak hash algorithms such as MD5 or SHA1.
Learn More
| # HIGH: Unsafe YAML loading (code execution) | ||
| def load_config(config_file): | ||
| with open(config_file, 'r') as f: | ||
| return yaml.load(f) |
There was a problem hiding this comment.
🔴 High: Code Vulnerability
| return yaml.load(f) | |
| return yaml.safe_load(f) |
prefer using safe_load to avoid arbitrary code execution (...read more)
Avoid deserialization of untrusted YAML data via potential unsafe yaml.load.
This rule checks that the yaml module is used and the load method is used. It recommends the usage of safe_load that prevents unsafe deserialization.
See Also
|
|
||
| // HIGH: XSS vulnerability (DOM-based) | ||
| function displayUserInput(input) { | ||
| document.getElementById('output').innerHTML = input; |
There was a problem hiding this comment.
🔴 High: Code Vulnerability
Updating HTML elements directly may lead to XSS vulnerabilities (...read more)
Properties like innerHTML and outerHTML should not be modified directly unless such modifications are clearly reviewed. Modifying innerHTML or outerHTML using user inputs that has not been validated can lead to XSS injection.
Learn More
|
|
||
| # HIGH: Command Injection vulnerability | ||
| def generate_report(report_name): | ||
| os.system("echo 'Report' > /tmp/" + report_name + ".txt") |
There was a problem hiding this comment.
⚫ Critical: Code Vulnerability
| os.system("echo 'Report' > /tmp/" + report_name + ".txt") | |
| os.system(shlex.quote("echo 'Report' > /tmp/" + report_name + ".txt")) |
unsanitized process call. Use shlex.quote() to sanitize the call (...read more)
Detect unsafe shell execution with the os module. We should ensure the command is safe before execution. Use shlex to sanitize user inputs.
Learn More
|
|
||
| // HIGH: SQL Injection vulnerability | ||
| function getUserById(userId) { | ||
| const query = "SELECT * FROM users WHERE id = " + userId; |
There was a problem hiding this comment.
⚫ Critical: Code Vulnerability
potential SQL injection (...read more)
Check for variable declarations in a SQL statement where there is potential for SQL injections.
Learn More
|
|
||
| // HIGH: SQL Injection in search | ||
| function searchProducts(searchTerm) { | ||
| const query = "SELECT * FROM products WHERE name LIKE '%" + searchTerm + "%'"; |
There was a problem hiding this comment.
⚫ Critical: Code Vulnerability
potential SQL injection (...read more)
Check for variable declarations in a SQL statement where there is potential for SQL injections.
Learn More
|
|
||
| # HIGH: Unsafe exec usage | ||
| def run_custom_logic(code): | ||
| exec(code) |
There was a problem hiding this comment.
⚫ Critical: Code Vulnerability
do not use exec as this is unsafe (...read more)
exec() is insecure, and passing in unsanitized data could create a vulnerability, as reported by the official Python documentation. Generated code should be controlled as mentioned in CWE-94.
Learn More
- CWE-94 - Improper Control of Generation of Code
| if user: | ||
| if user['active']: | ||
| if discount > 0: | ||
| if shipping: | ||
| if tax: | ||
| x = 1 | ||
| else: | ||
| x = 2 | ||
| else: | ||
| x = 3 | ||
| else: | ||
| x = 4 | ||
| else: | ||
| x = 5 | ||
| else: | ||
| x = 6 |
There was a problem hiding this comment.
🔴 High: Code Quality Violation
too many nesting levels (...read more)
Avoid to nest too many loops together. Having too many loops make your code harder to understand.
Prefer to organize your code in functions and unit of code you can clearly understand.
No description provided.