-
Notifications
You must be signed in to change notification settings - Fork 0
Add files via upload #11
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,48 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from flask import render_template | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import hashlib | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def sql_injection_login_page(request, app): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return render_template( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'sql_injection/login.html', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sql='', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logged=None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def sql_injection_login_api(request, app): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| form = request.form | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| username = form.get('username') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| password = form.get('password') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| password_hash = _hash_password(password) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sql = f"SELECT * FROM users WHERE username='{username}' AND password='{password_hash}'" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using the Django object-relational mappers (ORM) instead of raw SQL queries. View Dataflow Graphflowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>sql_injection_login.py</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L14 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 14] request.form</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L14 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 14] form</a>"]
v3["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L16 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 16] username</a>"]
end
v2 --> v3
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L20 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 20] f"SELECT * FROM users WHERE username='{username}' AND password='{password_hash}'"</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
⚪️ This finding does not block your pull request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Semgrep Assistant suggests the following fix: Use parameterized queries instead of string interpolation to prevent SQL injection. View step-by-step instructions
This code change should be a good starting point:
Suggested change
Leave feedback with a 👍 / 👎. Save a memory with |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sql = f"SELECT * FROM users WHERE username='{username}' AND password='{password_hash}'" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using the Django object-relational mappers (ORM) instead of raw SQL queries. View Dataflow Graphflowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>sql_injection_login.py</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L14 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 14] request.form</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L14 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 14] form</a>"]
v3["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L16 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 16] username</a>"]
end
v2 --> v3
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L21 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 21] f"SELECT * FROM users WHERE username='{username}' AND password='{password_hash}'"</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
⚪️ This finding does not block your pull request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Semgrep Assistant suggests the following fix: Use parameterized queries instead of manually constructing SQL strings. View step-by-step instructions
Alternatively, if you are using Django, consider using the Django ORM to handle database queries, which automatically uses parameterized queries. This code change should be a good starting point:
Suggested change
Leave feedback with a 👍 / 👎. Save a memory with |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sql = f"SELECT * FROM users WHERE username='{username}' AND password='{password_hash}'" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using the Django object-relational mappers (ORM) instead of raw SQL queries. View Dataflow Graphflowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>sql_injection_login.py</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L14 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 14] request.form</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L14 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 14] form</a>"]
v3["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L16 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 16] username</a>"]
end
v2 --> v3
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L22 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 22] f"SELECT * FROM users WHERE username='{username}' AND password='{password_hash}'"</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
⚪️ This finding does not block your pull request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Semgrep Assistant suggests the following fix: Use parameterized queries instead of manually constructing SQL strings. View step-by-step instructions
This code change should be a good starting point:
Suggested change
Leave feedback with a 👍 / 👎. Save a memory with |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sql = f"SELECT * FROM users WHERE username='{username}' AND password='{password_hash}'" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using the Django object-relational mappers (ORM) instead of raw SQL queries. View Dataflow Graphflowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>sql_injection_login.py</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L14 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 14] request.form</a>"]
end
%% Intermediate
subgraph Traces0[Traces]
direction TB
v2["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L14 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 14] form</a>"]
v3["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L16 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 16] username</a>"]
end
v2 --> v3
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L23 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 23] f"SELECT * FROM users WHERE username='{username}' AND password='{password_hash}'"</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
Traces0:::invis
File0:::invis
%% Connections
Source --> Traces0
Traces0 --> Sink
⚪️ This finding does not block your pull request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Semgrep Assistant suggests the following fix: Use parameterized queries instead of manually constructing SQL strings. View step-by-step instructions
This change ensures that user inputs are safely handled by the database engine, preventing SQL injection attacks. This code change should be a good starting point:
Suggested change
Leave feedback with a 👍 / 👎. Save a memory with |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| flask.render_template_string(username) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Found a template created with string formatting. This is susceptible to server-side template injection and cross-site scripting attacks. ⚪️ This finding does not block your pull request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Semgrep Assistant suggests the following fix: Use View step-by-step instructions
This code change should be a good starting point:
Suggested change
Leave feedback with a 👍 / 👎. Save a memory with |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db_result = app.db_helper.execute_read(sql) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user = list( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| map( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| lambda u: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'id': u[0], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'username': u[1], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'password': u[2] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| db_result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )[0] if len(db_result) > 0 else None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return render_template( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'sql_injection/login.html', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sql=sql, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logged=user is not None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _hash_password(password): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| md5_pass = hashlib.md5(password.encode('utf-8')).hexdigest() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Detected MD5 hash algorithm which is considered insecure. MD5 is not collision resistant and is therefore not suitable as a cryptographic signature. Use SHA256 or SHA3 instead. ⚪️ This finding does not block your pull request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Semgrep Assistant suggests the following fix: Use SHA-256 instead of MD5 for hashing passwords. View step-by-step instructions
Using SHA-256 provides better security because it is more resistant to collision attacks compared to MD5. This code change should be a good starting point:
Suggested change
Leave feedback with a 👍 / 👎. Save a memory with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like MD5 is used as a password hash. MD5 is not considered a secure password hash because it can be cracked by an attacker in a short amount of time. Use a suitable password hashing function such as scrypt. You can use View Dataflow Graphflowchart LR
classDef invis fill:white, stroke: none
classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none
subgraph File0["<b>sql_injection_login.py</b>"]
direction LR
%% Source
subgraph Source
direction LR
v0["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L47 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 47] hashlib.md5</a>"]
end
%% Intermediate
%% Sink
subgraph Sink
direction LR
v1["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L47 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 47] hashlib.md5(password.encode('utf-8')).hexdigest()</a>"]
end
end
%% Class Assignment
Source:::invis
Sink:::invis
File0:::invis
%% Connections
Source --> Sink
⚪️ This finding does not block your pull request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Semgrep Assistant suggests the following fix: Use View step-by-step instructions
This code change should be a good starting point:
Suggested change
Leave feedback with a 👍 / 👎. Save a memory with |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return md5_pass | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like MD5 is used as a password hash. MD5 is not considered a secure password hash because it can be cracked by an attacker in a short amount of time. Use a suitable password hashing function such as scrypt. You can use
hashlib.scrypt.View Dataflow Graph
flowchart LR classDef invis fill:white, stroke: none classDef default fill:#e7f5ff, color:#1c7fd6, stroke: none subgraph File0["<b>sql_injection_login.py</b>"] direction LR %% Source subgraph Source direction LR v0["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L47 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 47] hashlib.md5</a>"] end %% Intermediate subgraph Traces0[Traces] direction TB v2["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L47 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 47] md5_pass</a>"] v3["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L18 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 18] _hash_password</a>"] end v2 --> v3 %% Sink subgraph Sink direction LR v1["<a href=https://github.com/bandarisantosh/bad-python-app/commit/94974cb05250d6f66c19ce59611530fbf6942c86/sql_injection_login.py#L18 target=_blank style='text-decoration:none; color:#1c7fd6'>[Line: 18] _hash_password(password)</a>"] end end %% Class Assignment Source:::invis Sink:::invis Traces0:::invis File0:::invis %% Connections Source --> Traces0 Traces0 --> Sink⚪️ This finding does not block your pull request.
Ignore this finding from md5-used-as-password
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Semgrep Assistant suggests the following fix: Use
hashlib.scryptfor password hashing instead of MD5.View step-by-step instructions
hashlib.scryptfor password hashing._hash_passwordfunction to usehashlib.scryptinstead ofhashlib.md5. For example:hashlib.scryptwith the stored salt to check the password against the stored hash.This code change should be a good starting point:
Leave feedback with a 👍 / 👎. Save a memory with
/remember <your custom instructions>.