Skip to content

Semgrep files #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
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
4 changes: 4 additions & 0 deletions .coderabbit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
files:
include:
- "**/*.yml"
- "**/*.yaml"
Comment on lines +1 to +4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Invalid CodeRabbit configuration structure

The files property is not a valid top-level configuration option according to the CodeRabbit schema. Valid top-level properties include language, tone_instructions, early_access, enable_free_tier, reviews, chat, knowledge_base, and code_generation.

If you're trying to configure file filtering for reviews, use the reviews.path_filters property instead:

-files:
-  include:
-    - "**/*.yml"
-    - "**/*.yaml"
+reviews:
+  path_filters:
+    - "**/*.yml"
+    - "**/*.yaml"
🤖 Prompt for AI Agents
In the .coderabbit.yml file at lines 1 to 4, the top-level 'files' property is
invalid according to the CodeRabbit schema. To fix this, remove the 'files'
property and instead configure file filtering under the 'reviews.path_filters'
property by moving the include patterns there. Ensure the configuration uses
only valid top-level properties like 'language', 'tone_instructions', 'reviews',
etc.

32 changes: 32 additions & 0 deletions bad.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import os
import sys
import hashlib

# Hardcoded credentials
USERNAME = "admin"
PASSWORD = "secret123"
Comment on lines +6 to +7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical security issue: Hardcoded credentials detected.

Hardcoded credentials pose a significant security risk as they can be easily discovered in source code and cannot be rotated without code changes.

-USERNAME = "admin"
-PASSWORD = "secret123"
+# Use environment variables or secure configuration management
+USERNAME = os.getenv("APP_USERNAME")
+PASSWORD = os.getenv("APP_PASSWORD")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
USERNAME = "admin"
PASSWORD = "secret123"
# Use environment variables or secure configuration management
USERNAME = os.getenv("APP_USERNAME")
PASSWORD = os.getenv("APP_PASSWORD")
🤖 Prompt for AI Agents
In bad.py around lines 6 to 7, the USERNAME and PASSWORD are hardcoded, which is
a critical security risk. Remove these hardcoded credentials and instead load
them securely from environment variables or a secure secrets manager. Update the
code to read USERNAME and PASSWORD from these external sources to allow secure
credential management and rotation without code changes.


def dangerous_eval():
user_input = input("Enter a Python expression: ")
result = eval(user_input)
print("Evaluated result:", result)
Comment on lines +9 to +12
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical security vulnerability: Code injection via eval().

Using eval() on user input allows arbitrary code execution, which is a severe security vulnerability that can lead to system compromise.

-def dangerous_eval():
-    user_input = input("Enter a Python expression: ")
-    result = eval(user_input)
-    print("Evaluated result:", result)
+def safe_calculation():
+    try:
+        user_input = input("Enter a mathematical expression: ")
+        # Use ast.literal_eval() for safe evaluation of literals only
+        # or implement a proper expression parser
+        result = ast.literal_eval(user_input)
+        print("Evaluated result:", result)
+    except (ValueError, SyntaxError) as e:
+        print("Invalid input:", e)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def dangerous_eval():
user_input = input("Enter a Python expression: ")
result = eval(user_input)
print("Evaluated result:", result)
def safe_calculation():
try:
user_input = input("Enter a mathematical expression: ")
# Use ast.literal_eval() for safe evaluation of literals only
# or implement a proper expression parser
result = ast.literal_eval(user_input)
print("Evaluated result:", result)
except (ValueError, SyntaxError) as e:
print("Invalid input:", e)
🤖 Prompt for AI Agents
In bad.py around lines 9 to 12, the use of eval() on user input creates a
critical security vulnerability by allowing arbitrary code execution. Replace
eval() with a safer alternative such as ast.literal_eval() if evaluating
expressions, or implement strict input validation and parsing to avoid executing
arbitrary code. Ensure that user input is never directly executed as code.


def delete_data(path):
os.system("rm -rf " + path) # Semgrep: shell injection
Comment on lines +14 to +15
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical security vulnerability: Shell injection detected.

Direct concatenation of user input into shell commands allows command injection attacks.

-def delete_data(path):
-    os.system("rm -rf " + path)  # Semgrep: shell injection
+def delete_data(path):
+    import shutil
+    # Validate and sanitize the path
+    if os.path.exists(path) and os.path.abspath(path).startswith('/safe/directory/'):
+        shutil.rmtree(path)
+    else:
+        raise ValueError("Invalid or unsafe path")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def delete_data(path):
os.system("rm -rf " + path) # Semgrep: shell injection
def delete_data(path):
import shutil
# Validate and sanitize the path
if os.path.exists(path) and os.path.abspath(path).startswith('/safe/directory/'):
shutil.rmtree(path)
else:
raise ValueError("Invalid or unsafe path")
🤖 Prompt for AI Agents
In bad.py around lines 14 to 15, the function delete_data uses os.system with
direct string concatenation of the path argument, which creates a shell
injection vulnerability. To fix this, avoid using os.system with concatenated
strings; instead, use the subprocess module with a list of arguments or use
Python's built-in file operations to safely delete files or directories without
invoking the shell.


def hash_password(password):
hashed = hashlib.md5(password.encode()).hexdigest() # Semgrep: weak hash
return hashed
Comment on lines +17 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Security issue: Weak cryptographic hashing algorithm.

MD5 is cryptographically broken and should not be used for password hashing. Use a proper password hashing library instead.

-def hash_password(password):
-    hashed = hashlib.md5(password.encode()).hexdigest()  # Semgrep: weak hash
-    return hashed
+def hash_password(password):
+    import bcrypt
+    # Use bcrypt for secure password hashing
+    salt = bcrypt.gensalt()
+    hashed = bcrypt.hashpw(password.encode('utf-8'), salt)
+    return hashed
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def hash_password(password):
hashed = hashlib.md5(password.encode()).hexdigest() # Semgrep: weak hash
return hashed
def hash_password(password):
import bcrypt
# Use bcrypt for secure password hashing
salt = bcrypt.gensalt()
hashed = bcrypt.hashpw(password.encode('utf-8'), salt)
return hashed
🤖 Prompt for AI Agents
In bad.py around lines 17 to 19, the function uses MD5 for hashing passwords,
which is insecure. Replace the MD5 hashing with a secure password hashing
library such as bcrypt or Argon2. Update the function to use the chosen
library's hashing method and ensure the password is hashed with a strong,
adaptive algorithm designed for password storage.


def main():
print("Logging in as", USERNAME)
password_hash = hash_password(PASSWORD)
print("Password hash:", password_hash)

if len(sys.argv) > 1:
delete_data(sys.argv[1])
Comment on lines +26 to +27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Security issue: Unsafe command line argument handling.

Directly passing command line arguments to potentially dangerous functions without validation creates security risks.

-    if len(sys.argv) > 1:
-        delete_data(sys.argv[1])
+    if len(sys.argv) > 1:
+        path = sys.argv[1]
+        # Validate the path before processing
+        if validate_path(path):
+            delete_data(path)
+        else:
+            print("Error: Invalid path provided")

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In bad.py around lines 26 to 27, the code directly passes a command line
argument to the delete_data function without any validation, posing a security
risk. To fix this, add proper validation or sanitization of sys.argv[1] before
passing it to delete_data, ensuring it meets expected criteria and does not
contain harmful input.


dangerous_eval()

main()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding proper entry point protection.

The script executes immediately when imported, which can cause issues in testing or when used as a module.

-main()
+if __name__ == "__main__":
+    main()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
main()
if __name__ == "__main__":
main()
🤖 Prompt for AI Agents
In bad.py at line 31, the script calls main() directly, causing it to execute
upon import. To fix this, wrap the main() call inside a standard Python entry
point check by adding if __name__ == "__main__": before calling main(). This
ensures the script only runs when executed directly, not when imported as a
module.


8 changes: 8 additions & 0 deletions semgrep.config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
rules:
- id: hardcoded-password
pattern: password = "$SECRET"
message: "Avoid hardcoded passwords"
severity: ERROR
languages: [python]
metadata:
category: security
Comment on lines +1 to +8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve the hardcoded password detection pattern

The current pattern password = "$SECRET" is too specific and won't detect the actual hardcoded password in bad.py (PASSWORD = "secret123"). Consider using a more comprehensive pattern:

 rules:
   - id: hardcoded-password
-    pattern: password = "$SECRET"
+    patterns:
+      - pattern-either:
+          - pattern: $VAR = "..."
+          - pattern: $VAR = '...'
+      - metavariable-regex:
+          metavariable: $VAR
+          regex: '(?i)(password|passwd|pwd|pass|secret|api_key|apikey|auth|token)(_|$)'
+      - pattern-not: $VAR = None
+      - pattern-not: $VAR = ""
     message: "Avoid hardcoded passwords"
     severity: ERROR
     languages: [python]
     metadata:
       category: security

This enhanced pattern will catch various forms of hardcoded credentials including the ones in bad.py.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
rules:
- id: hardcoded-password
pattern: password = "$SECRET"
message: "Avoid hardcoded passwords"
severity: ERROR
languages: [python]
metadata:
category: security
rules:
- id: hardcoded-password
patterns:
- pattern-either:
- pattern: $VAR = "..."
- pattern: $VAR = '...'
- metavariable-regex:
metavariable: $VAR
regex: '(?i)(password|passwd|pwd|pass|secret|api_key|apikey|auth|token)(_|$)'
- pattern-not: $VAR = None
- pattern-not: $VAR = ""
message: "Avoid hardcoded passwords"
severity: ERROR
languages: [python]
metadata:
category: security
🤖 Prompt for AI Agents
In semgrep.config.yml lines 1 to 8, the current pattern for detecting hardcoded
passwords is too specific and misses cases like PASSWORD = "secret123". Update
the pattern to a more general one that matches any assignment to variables named
password or similar with string literals, covering different cases and
variations. This will improve detection of hardcoded credentials in Python code.