-
Notifications
You must be signed in to change notification settings - Fork 1
Create user.py #70
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?
Create user.py #70
Conversation
| password = request.args.get("password") | ||
|
|
||
| conn = sqlite3.connect("users.db") | ||
|
|
||
| # CHANGE: unsafe query construction (SQL Injection) | ||
| sql = "SELECT * FROM users WHERE username = '%s' AND password = '%s'" % (username, password) | ||
| conn.execute(sql) |
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.
Static Code Analysis Risk: Injection - SQL injection DB cursor execute
User-controlled data from a request is passed to 'execute()'. This could lead to a SQL injection and therefore protected information could be leaked. Instead, use django's QuerySets, which are built with query parameterization and therefore not vulnerable to sql injection. For example, you could use Entry.objects.filter(date=2006).
Severity: Medium
Status: Open 🔴
References:
Suggested reviewers 🧐: @RZB1088
More details:
Details
Take action by replying with an [arnica] command 💬
Actions
Use [arnica] or [a] to interact with the Arnica bot to acknowledge or dismiss code risks.
To acknowledge the finding as a valid code risk:
[arnica] ack <acknowledge additional details>
To dismiss the risk with a reason:
[arnica] dismiss <fp|accept|capacity> <dismissal reason>
Examples
-
[arnica] ack This is a valid risk and im looking into it -
[arnica] dismiss fp Dismissed - Risk Not Accurate: (i.e. False Positive) -
[arnica] dismiss accept Dismiss - Risk Accepted: Allow the risk to exist in the system -
[arnica] dismiss capacity Dismiss - No Capacity: This will need to wait for a future sprint
| username = request.args.get("username") | ||
| password = request.args.get("password") | ||
|
|
||
| conn = sqlite3.connect("users.db") | ||
|
|
||
| # CHANGE: unsafe query construction (SQL Injection) | ||
| sql = "SELECT * FROM users WHERE username = '%s' AND password = '%s'" % (username, 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.
Static Code Analysis Risk: Software and Data Integrity Failures - Tainted SQL string
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.
Severity: Low ⬇️
Status: Open 🔴
References:
Suggested reviewers 🧐: @RZB1088
More details:
Details
Take action by replying with an [arnica] command 💬
Actions
Use [arnica] or [a] to interact with the Arnica bot to acknowledge or dismiss code risks.
To acknowledge the finding as a valid code risk:
[arnica] ack <acknowledge additional details>
To dismiss the risk with a reason:
[arnica] dismiss <fp|accept|capacity> <dismissal reason>
Examples
-
[arnica] ack This is a valid risk and im looking into it -
[arnica] dismiss fp Dismissed - Risk Not Accurate: (i.e. False Positive) -
[arnica] dismiss accept Dismiss - Risk Accepted: Allow the risk to exist in the system -
[arnica] dismiss capacity Dismiss - No Capacity: This will need to wait for a future sprint
| username = request.args.get("username") | ||
| password = request.args.get("password") | ||
|
|
||
| conn = sqlite3.connect("users.db") | ||
|
|
||
| # CHANGE: unsafe query construction (SQL Injection) | ||
| sql = "SELECT * FROM users WHERE username = '%s' AND password = '%s'" % (username, 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.
Static Code Analysis Risk: Injection - Tainted SQL string
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 an object-relational mapper (ORM) such as SQLAlchemy which will protect your queries.
Severity: Medium
Status: Open 🔴
References:
- https://docs.sqlalchemy.org/en/14/core/tutorial.html#using-textual-sql
- https://www.tutorialspoint.com/sqlalchemy/sqlalchemy_quick_guide.htm
- https://docs.sqlalchemy.org/en/14/core/tutorial.html#using-more-specific-text-with-table-expression-literal-column-and-expression-column
Suggested reviewers 🧐: @RZB1088
More details:
Details
Take action by replying with an [arnica] command 💬
Actions
Use [arnica] or [a] to interact with the Arnica bot to acknowledge or dismiss code risks.
To acknowledge the finding as a valid code risk:
[arnica] ack <acknowledge additional details>
To dismiss the risk with a reason:
[arnica] dismiss <fp|accept|capacity> <dismissal reason>
Examples
-
[arnica] ack This is a valid risk and im looking into it -
[arnica] dismiss fp Dismissed - Risk Not Accurate: (i.e. False Positive) -
[arnica] dismiss accept Dismiss - Risk Accepted: Allow the risk to exist in the system -
[arnica] dismiss capacity Dismiss - No Capacity: This will need to wait for a future sprint
| username = request.args.get("username") | ||
| password = request.args.get("password") | ||
|
|
||
| conn = sqlite3.connect("users.db") | ||
|
|
||
| # CHANGE: unsafe query construction (SQL Injection) | ||
| sql = "SELECT * FROM users WHERE username = '%s' AND password = '%s'" % (username, password) | ||
| conn.execute(sql) |
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.
Static Code Analysis Risk: Injection - SQL injection DB cursor execute
User-controlled data from a request is passed to 'execute()'. This could lead to a SQL injection and therefore protected information could be leaked. Instead, use django's QuerySets, which are built with query parameterization and therefore not vulnerable to sql injection. For example, you could use Entry.objects.filter(date=2006).
Severity: Medium
Status: Open 🔴
References:
Suggested reviewers 🧐: @RZB1088
More details:
Details
Take action by replying with an [arnica] command 💬
Actions
Use [arnica] or [a] to interact with the Arnica bot to acknowledge or dismiss code risks.
To acknowledge the finding as a valid code risk:
[arnica] ack <acknowledge additional details>
To dismiss the risk with a reason:
[arnica] dismiss <fp|accept|capacity> <dismissal reason>
Examples
-
[arnica] ack This is a valid risk and im looking into it -
[arnica] dismiss fp Dismissed - Risk Not Accurate: (i.e. False Positive) -
[arnica] dismiss accept Dismiss - Risk Accepted: Allow the risk to exist in the system -
[arnica] dismiss capacity Dismiss - No Capacity: This will need to wait for a future sprint
| host = request.args.get("host") | ||
|
|
||
| # CHANGE: user input passed directly to OS command (Command Injection) | ||
| command = "ping -c 1 " + host | ||
| os.system(command) |
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.
Static Code Analysis Risk: Injection - Command injection OS system
Request data detected in os.system. This could be vulnerable to a command injection and should be avoided. If this must be done, use the 'subprocess' module instead and pass the arguments as a list. See https://owasp.org/www-community/attacks/Command_Injection for more information.
Severity: Medium
Status: Open 🔴
References:
Suggested reviewers 🧐: @RZB1088
More details:
Details
Take action by replying with an [arnica] command 💬
Actions
Use [arnica] or [a] to interact with the Arnica bot to acknowledge or dismiss code risks.
To acknowledge the finding as a valid code risk:
[arnica] ack <acknowledge additional details>
To dismiss the risk with a reason:
[arnica] dismiss <fp|accept|capacity> <dismissal reason>
Examples
-
[arnica] ack This is a valid risk and im looking into it -
[arnica] dismiss fp Dismissed - Risk Not Accurate: (i.e. False Positive) -
[arnica] dismiss accept Dismiss - Risk Accepted: Allow the risk to exist in the system -
[arnica] dismiss capacity Dismiss - No Capacity: This will need to wait for a future sprint
|
|
||
| # CHANGE: unsafe query construction (SQL Injection) | ||
| sql = "SELECT * FROM users WHERE username = '%s' AND password = '%s'" % (username, password) | ||
| conn.execute(sql) |
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.
Static Code Analysis Risk: Injection - Formatted SQL query
Detected possible formatted SQL query. Use parameterized queries instead.
Severity: Low ⬇️
Status: Open 🔴
References:
Suggested reviewers 🧐: @RZB1088
More details:
Details
Take action by replying with an [arnica] command 💬
Actions
Use [arnica] or [a] to interact with the Arnica bot to acknowledge or dismiss code risks.
To acknowledge the finding as a valid code risk:
[arnica] ack <acknowledge additional details>
To dismiss the risk with a reason:
[arnica] dismiss <fp|accept|capacity> <dismissal reason>
Examples
-
[arnica] ack This is a valid risk and im looking into it -
[arnica] dismiss fp Dismissed - Risk Not Accurate: (i.e. False Positive) -
[arnica] dismiss accept Dismiss - Risk Accepted: Allow the risk to exist in the system -
[arnica] dismiss capacity Dismiss - No Capacity: This will need to wait for a future sprint
|
|
||
| # CHANGE: unsafe query construction (SQL Injection) | ||
| sql = "SELECT * FROM users WHERE username = '%s' AND password = '%s'" % (username, password) | ||
| conn.execute(sql) |
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.
Static Code Analysis Risk: Injection - Sqlalchemy execute raw query
Avoiding SQL string concatenation: untrusted input concatenated with raw SQL query can result in SQL Injection. In order to execute raw query safely, prepared statement should be used. SQLAlchemy provides TextualSQL to easily used prepared statement with named parameters. For complex SQL composition, use SQL Expression Language or Schema Definition Language. In most cases, SQLAlchemy ORM will be a better option.
Severity: Low ⬇️
Status: Open 🔴
References:
- https://docs.sqlalchemy.org/en/14/core/tutorial.html#using-textual-sql
- https://www.tutorialspoint.com/sqlalchemy/sqlalchemy_quick_guide.htm
- https://docs.sqlalchemy.org/en/14/core/tutorial.html#using-more-specific-text-with-table-expression-literal-column-and-expression-column
Suggested reviewers 🧐: @RZB1088
More details:
Details
Take action by replying with an [arnica] command 💬
Actions
Use [arnica] or [a] to interact with the Arnica bot to acknowledge or dismiss code risks.
To acknowledge the finding as a valid code risk:
[arnica] ack <acknowledge additional details>
To dismiss the risk with a reason:
[arnica] dismiss <fp|accept|capacity> <dismissal reason>
Examples
-
[arnica] ack This is a valid risk and im looking into it -
[arnica] dismiss fp Dismissed - Risk Not Accurate: (i.e. False Positive) -
[arnica] dismiss accept Dismiss - Risk Accepted: Allow the risk to exist in the system -
[arnica] dismiss capacity Dismiss - No Capacity: This will need to wait for a future sprint
No description provided.