Skip to content

Update hello.py - #88

Open
yash-deepsource wants to merge 1 commit into
masterfrom
yash-deepsource-patch-7
Open

Update hello.py#88
yash-deepsource wants to merge 1 commit into
masterfrom
yash-deepsource-patch-7

Conversation

@yash-deepsource

Copy link
Copy Markdown

No description provided.

@deepsource-development

deepsource-development Bot commented Apr 3, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 9d1323c...8d6c8ae on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade  

Focus Area: Security
Security  

Reliability  

Complexity  

Hygiene  

Feedback

Deliberate insecurity across layers

  • Almost every security issue here is “by design” for the SAST demo: hardcoded secret, unsafe SQL, MD5, pickle, eval, and shell=True are all aligned around showcasing common injection/compromise vectors.
  • That’s good for coverage, but it may be worth thinking about how to keep this clearly quarantined from anything real so it never gets reused by accident.

Code Review Summary

Analyzer Status Updated (UTC) Details
Python Apr 3, 2026 9:38p.m. Review ↗
Secrets Apr 3, 2026 9:38p.m. Review ↗

Comment thread hello.py
# Hard-coded secret (SAST should flag this)
API_KEY = "AKIAEXAMPLEHARDCODEDKEY123456"

DB_PATH = "/tmp/demo_app.db"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Probable insecure usage of temp file/directory.


Using hardcoded temp directory is unsafe. The program can be tricked into performing file actions against the wrong file or using a malicious file instead of the expected temporary file. Prefer using tempfile

Comment thread hello.py
try:
cur = conn.cursor()
# Insecure SQL construction; vulnerable to SQL injection if username contains malicious payload.
sql = f"INSERT INTO users (username, password_hash, profile_blob) VALUES ('{username}', '{password_hash}', ?)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Possible SQL injection vector through string-based query construction.


Constructing SQL query using user provided data is insecure. It makes application vulnerable to [SQL injection](SQL injection) attacks.

Comment thread hello.py
try:
cur = conn.cursor()
# Insecure: SQL built using string formatting
sql = "SELECT password_hash FROM users WHERE username = '%s'" % username

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Formatting a regular string which could be a f-string


f-strings are the fastest way to format strings as compared to the following methods: * using format specifiers %

Comment thread hello.py
try:
cur = conn.cursor()
# Insecure: SQL built using string formatting
sql = "SELECT password_hash FROM users WHERE username = '%s'" % username

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Possible SQL injection vector through string-based query construction.


Constructing SQL query using user provided data is insecure. It makes application vulnerable to [SQL injection](SQL injection) attacks.

Comment thread hello.py
return None
blob = row[0]
# Insecure: untrusted pickle.loads
profile = pickle.loads(blob)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pickle and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue.


The pickle module is not secure against erroneous or maliciously constructed data. Never unpickle data received from an untrusted or unauthenticated source.

Comment thread hello.py
finally:
conn.close()

def _weak_hash(self, value: str) -> str:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Method doesn't use the class instance and could be converted into a static method


The method doesn't use its bound instance. Decorate this method with @staticmethod decorator, so that Python does not have to instantiate a bound method for every instance of this class thereby saving memory and computation. Read more about staticmethods here.

Comment thread hello.py
Weak hashing function (MD5) used for historical compatibility.
SAST should flag use of insecure hashing algorithms for credentials.
"""
h = hashlib.md5()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use of insecure hashlib.md5 hash function


D2, MD4, MD5, SHA1 signature algorithms are known to be vulnerable to collision attacks. Attackers can exploit this to generate another certificate with the same digital signature, allowing them to masquerade as the affected service.

Comment thread hello.py
# Logging user-provided command (may contain sensitive data)
logger.debug("Running system check: %s", cmd)
# Insecure: shell=True and direct command interpolation
result = subprocess.check_output(cmd, shell=True, stderr=subprocess.STDOUT, text=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

subprocess call with shell=True identified, security issue.


Using shell=True can expose you to security risks if someone crafts input to issue different commands than the ones you intended.

Comment thread hello.py
"""
logger.debug("Evaluating config string.")
# Insecure: direct eval of input
return eval(config_str)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Use of eval


Use of possibly insecure function - consider using safer ast.literal_eval. Read more on why should eval be avoided here.

@vishnu-deepsource

Copy link
Copy Markdown

@deepsourcebot Yo, run an analysis with AI!

@vishnu-deepsource

Copy link
Copy Markdown

@deepsourcebot review this pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants