Skip to content
Open
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
28 changes: 28 additions & 0 deletions python/user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from flask import Flask, request
import sqlite3
import os

app = Flask(__name__)

@app.route("/login")
def login():
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)
Comment on lines +9 to +15

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:

  1. https://docs.djangoproject.com/en/3.0/topics/security/#sql-injection-protection

Suggested reviewers 🧐: @RZB1088

More details:

🌻 View in Arnica


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

Comment on lines +9 to +15

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:

  1. https://docs.sqlalchemy.org/en/14/core/tutorial.html#using-textual-sql
  2. https://www.tutorialspoint.com/sqlalchemy/sqlalchemy_quick_guide.htm
  3. 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:

🌻 View in Arnica


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

conn.execute(sql)
Comment on lines +10 to +16

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:

  1. https://docs.djangoproject.com/en/3.0/topics/security/#sql-injection-protection

Suggested reviewers 🧐: @RZB1088

More details:

🌻 View in Arnica


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

Comment on lines +9 to +16

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:

  1. https://docs.djangoproject.com/en/3.0/topics/security/#sql-injection-protection

Suggested reviewers 🧐: @RZB1088

More details:

🌻 View in Arnica


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

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:

  1. https://stackoverflow.com/questions/775296/mysql-parameterized-queries

Suggested reviewers 🧐: @RZB1088

More details:

🌻 View in Arnica


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

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:

  1. https://docs.sqlalchemy.org/en/14/core/tutorial.html#using-textual-sql
  2. https://www.tutorialspoint.com/sqlalchemy/sqlalchemy_quick_guide.htm
  3. 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:

🌻 View in Arnica


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


return "Logged in"

@app.route("/ping")
def ping():
host = request.args.get("host")

# CHANGE: user input passed directly to OS command (Command Injection)
command = "ping -c 1 " + host
os.system(command)
Comment on lines +22 to +26

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:

  1. https://owasp.org/www-community/attacks/Command_Injection

Suggested reviewers 🧐: @RZB1088

More details:

🌻 View in Arnica


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


return "Ping sent"