From a269d4a9ce057e0cda149650da180e8adfa7b7e5 Mon Sep 17 00:00:00 2001 From: Nicolas Valcarcel Date: Fri, 19 Jul 2019 12:28:21 -0500 Subject: [PATCH 1/3] Add user intake form --- README.md | 148 ++-------------------------------------- marketplace/__init__.py | 3 +- marketplace/models.py | 7 ++ marketplace/users.py | 20 ++++++ 4 files changed, 34 insertions(+), 144 deletions(-) create mode 100644 marketplace/users.py diff --git a/README.md b/README.md index 0210613..078e7f2 100644 --- a/README.md +++ b/README.md @@ -1,154 +1,16 @@ # Secure Coding with Python. -## Chapter 2: SQL Injection +## Chapter 3: Weak Password Storage ### Requirement -Since we are creating a marketplace application, we first decide to allow the upload of Listings, just text. We will worry about users later, since we want to focus on getting the DB and Models setup without needed to worry about authentication and session management at this point. +Now that we know our DB is working, it's time to start creating some users. We should have a signup account that create the user. ### Development -Since the application will need some more configuration we change the `marketplace/__init__.py` to make use of the `create_app` factory function. We add the DB connection functions into `marketplace/db.py` and add the factory function. We also add the DB schema in `schema.sql` and add a flask command to init the DB, which we run with the `python -m flask init-db` command. +We create a signup page, a user model and start taking in new users. ### Vulnerability -Since we are generating the SQL to insert the new listing in a very unsecure way, we can insert SQL commands that will be run in the DB. For example if we insert `'` as title or description we will get `psycopg2.errors.SyntaxError: INSERT has more target columns than expressions LINE 1: INSERT INTO listings (title, description) VALUES (''', ''') ^` instead of a success. +Since we are not thoughtful on what we are doing, we are storing the passwords in plain text. Meaning anyone with access to our DB, or exploiting an SQL injection, as shown in previous chapter, can easily get any user password. -We can for example get the postgresql version or any other SQL function result, to check that out, insert `injection', (select version()))-- -` as the title. When we do so, the SQL that's going to be executed will be the following: -```sql -INSERT INTO listings (title, description) VALUES ('injection', (select version()))-- -', 'ignored description') -``` -As it can be seen, the inserted title will be `injection` and the description will be the result of the `select version()` command, or any other command we wish to insert there, including dropping the DB. - -### Testing -Testing for SQL injections is a tedious job, it's mostly done by hand or using special scanners, like web scanners or SAST/DAST tools. For this chapter we will be writing a very simple fuzzer function and create unit tests that use them in order to test for injections. - -The fuzzer helper looks like this: -```python -import pytest - -from psycopg2.errors import SyntaxError - -def sqli_fuzzer(client, url, params): - fail = False - injections = ["'"] - for injection in injections: - for param in params: - data = {k: 'foo' for k in params} - data[param] = injection - try: - client.post(url, data=data) - except SyntaxError: - print('You seems to have an SQLi in %s for param %s' % (url, param)) - fail = True - - if fail: - pytest.fail('Seems you are vulnerable to SQLi attacks') -``` - -After running `pytest --tb=short` we get: -```text -============================= test session starts ============================== -platform linux -- Python 3.5.3, pytest-5.0.1, py-1.8.0, pluggy-0.12.0 -rootdir: {...} -collected 1 item - -tests/test_listings.py F [100%] - -=================================== FAILURES =================================== -_________________________________ test_create __________________________________ -tests/test_listings.py:6: in test_create - sqli_fuzzer(client, '/listings/create', ['title', 'description']) -tests/helpers/sqlifuzzer.py:19: in sqli_fuzzer - pytest.fail('Seems you are vulnerable to SQLi attacks') -E Failed: Seems you are vulnerable to SQLi attacks ------------------------------ Captured stdout call ----------------------------- -INSERT INTO listings (title, description) VALUES (''', 'foo') -You seems to have an SQLi in /listings/create for param title -INSERT INTO listings (title, description) VALUES ('foo', ''') -You seems to have an SQLi in /listings/create for param description -=========================== 1 failed in 0.32 seconds =========================== - -``` -### Fix -Given that we have seen that the way this injection works is by breaking out of the `'`'s, we can use PostgreSQL escaping `E'\''`. For that we change our SQL query and replace every occurrence of `'` with `\'`: -```python - sql = "INSERT INTO listings (title, description) VALUES (E'%s', E'%s')" % ( - title.replace("'", "\\'"), description.replace("'", "\\'") - ) -``` - -With that our test now pass: -```text -(venv) > $ pytest --tb=short -================================================================================================== test session starts =================================================================================================== -platform linux -- Python 3.5.3, pytest-5.0.0, py-1.8.0, pluggy-0.12.0 -rootdir: {...} -collected 1 item -tests/test_listings.py . [100%] -================================================================================================ 1 passed in 0.95 seconds ================================================================================================ -``` - -But this is not sufficient, if we modify our payload to be `injection\', (select version()))-- -` our query will end up being: -```sql -INSERT INTO listings (title, description) VALUES (E'injection\\', (select version()))-- -', E'\'') -``` -and attacker will still be able to exploit our app. - -### Testing part 2 -We could keep adding more cases to our fuzzer, or use external tools, like [sqlmap](http://sqlmap.org/), which are going to be limited by the test cases we can pass to them, we could also use a Static Application Security Testing, like [bandit](https://github.com/PyCQA/bandit/). - -```text -(venv) > $ bandit marketplace/**/*.py -Test results: ->> Issue: [B608:hardcoded_sql_expressions] Possible SQL injection vector through string-based query construction. - Severity: Medium Confidence: Low - Location: marketplace/listings.py:27 - More Info: https://bandit.readthedocs.io/en/latest/plugins/b608_hardcoded_sql_expressions.html -26 -27 sql = "INSERT INTO listings (title, description) VALUES (E'%s', E'%s')" % ( -28 title.replace("'", "\\'"), description.replace("'", "\\'") -29 ) - --------------------------------------------------- - -Code scanned: - Total lines of code: 28 - Total lines skipped (#nosec): 0 - -Run metrics: - Total issues (by severity): - Undefined: 0.0 - Low: 0.0 - Medium: 1.0 - High: 0.0 - Total issues (by confidence): - Undefined: 0.0 - Low: 1.0 - Medium: 0.0 - High: 0.0 -Files skipped (0): -``` -As we can see, the tool doesn't like our sanitization strategies and flags our code as a possible source of SQL injection. - -### Fix part 2 -In order to fix the SQL injetion once and for all, we should rely on prepared statements, and let the DB engine do the param sanitization, like this: -```python - sql = "INSERT INTO listings (title, description) VALUES (%s, %s)" - cur.execute(sql, (title, description)) -``` - -Now both our unit test and bandit are happy! - -### Fix part 3 -An even better approach is to use an ORM, in this case we set up SQLAlchemy, by using the standard methods the ORM will do the sanitization so we don't need to worry about it. - -**Note**: Most ORMs in some special use cases can still allow SQL Injections to happen, if you are using non-standard methods, review the ORMs security guidelines and test your application. - -## Description -Welcome to the Secure coding with python course. In this repository you will find a series of branches for each step of the development of a sample marketplace application. In such a development, we will be making security mistakes and introducing vulnerabilities, we will add tests for them and finally fixing them. - -The branches will have the following naming scheme for easier navigation: {Chapter number}-{Chapter Name}/{code|test|fix}. I encourage you to follow the chapters in order, but you can also skip to the specific one you wish to review. - -For this course we will be using Python3, Flask and PostgreSQL. - -**Proceed to [next section](https://github.com/nxvl/secure-coding-with-python/tree/3.1-weak-password-storage/code)** +**Proceed to [next section](https://github.com/nxvl/secure-coding-with-python/tree/3.1-weak-password-storage/fix)** ## Index ### 1. Vulnerable Components diff --git a/marketplace/__init__.py b/marketplace/__init__.py index 3b1896e..d4a8872 100644 --- a/marketplace/__init__.py +++ b/marketplace/__init__.py @@ -25,7 +25,8 @@ def create_app(test_config=None): db.init_app(app) migrate.init_app(app, db) - from . import listings + from . import listings, users app.register_blueprint(listings.bp) + app.register_blueprint(users.bp) return app \ No newline at end of file diff --git a/marketplace/models.py b/marketplace/models.py index f234b22..1161df4 100644 --- a/marketplace/models.py +++ b/marketplace/models.py @@ -1,5 +1,12 @@ from . import db +class User(db.Model): + __tablename__ = 'users' + id = db.Column(db.Integer, primary_key=True) + full_name = db.Column(db.String(100)) + email = db.Column(db.String(100)) + password = db.Column(db.String(100)) + class Listing(db.Model): __tablename__ = 'listings' diff --git a/marketplace/users.py b/marketplace/users.py new file mode 100644 index 0000000..3d7e54f --- /dev/null +++ b/marketplace/users.py @@ -0,0 +1,20 @@ +from flask import Blueprint, request, render_template + +from . import db +from .models import User + +bp = Blueprint('users', __name__, url_prefix='/user') + +@bp.route('/signup', methods=('GET', 'POST')) +def sign_up(): + if request.method == 'POST': + user = User( + full_name=request.form['full_name'], + email=request.form['email'], + password=request.form['password'], + ) + db.session.add(user) + db.session.commit() + + return render_template('users/signup_success.html', user=user) + return render_template('users/signup.html') \ No newline at end of file From 342b56078346d4423d510dd74f63a46aa819c619 Mon Sep 17 00:00:00 2001 From: Nicolas Valcarcel Date: Fri, 19 Jul 2019 12:28:32 -0500 Subject: [PATCH 2/3] added missing files --- marketplace/templates/users/signup.html | 17 ++++++++++ .../templates/users/signup_success.html | 9 +++++ migrations/versions/67168ab4efaa_.py | 34 +++++++++++++++++++ 3 files changed, 60 insertions(+) create mode 100644 marketplace/templates/users/signup.html create mode 100644 marketplace/templates/users/signup_success.html create mode 100644 migrations/versions/67168ab4efaa_.py diff --git a/marketplace/templates/users/signup.html b/marketplace/templates/users/signup.html new file mode 100644 index 0000000..0e7d108 --- /dev/null +++ b/marketplace/templates/users/signup.html @@ -0,0 +1,17 @@ +{% extends 'base.html' %} + +{% block header %} +

{% block title %}Create new user{% endblock %}

+{% endblock %} + +{% block content %} +
+ + + + + + + +
+{% endblock %} \ No newline at end of file diff --git a/marketplace/templates/users/signup_success.html b/marketplace/templates/users/signup_success.html new file mode 100644 index 0000000..d61c5ac --- /dev/null +++ b/marketplace/templates/users/signup_success.html @@ -0,0 +1,9 @@ +{% extends 'base.html' %} + +{% block header %} +

{% block title %}Successful user creation{% endblock %}

+{% endblock %} + +{% block content %} + Successfully created user for {{ user.full_name }} with email {{ user.email }} +{% endblock %} \ No newline at end of file diff --git a/migrations/versions/67168ab4efaa_.py b/migrations/versions/67168ab4efaa_.py new file mode 100644 index 0000000..0249bb8 --- /dev/null +++ b/migrations/versions/67168ab4efaa_.py @@ -0,0 +1,34 @@ +"""empty message + +Revision ID: 67168ab4efaa +Revises: d018d799acf7 +Create Date: 2019-07-19 12:26:57.013800 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '67168ab4efaa' +down_revision = 'd018d799acf7' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('users', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('full_name', sa.String(length=100), nullable=True), + sa.Column('email', sa.String(length=100), nullable=True), + sa.Column('password', sa.String(length=100), nullable=True), + sa.PrimaryKeyConstraint('id') + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table('users') + # ### end Alembic commands ### From bd077c3211a5e26d7e6f79d7ca9f7ee47733ce3f Mon Sep 17 00:00:00 2001 From: Nicolas Valcarcel Date: Sat, 3 Aug 2019 15:49:04 -0500 Subject: [PATCH 3/3] clean README --- README.md | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/README.md b/README.md index 3d3331d..d1f262c 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,5 @@ # Secure Coding with Python. -<<<<<<< HEAD ## Chapter 3: Weak Password Storage ### Requirement Now that we know our DB is working, it's time to start creating some users. We should have a signup account that create the user. @@ -12,22 +11,6 @@ We create a signup page, a user model and start taking in new users. Since we are not thoughtful on what we are doing, we are storing the passwords in plain text. Meaning anyone with access to our DB, or exploiting an SQL injection, as shown in previous chapter, can easily get any user password. **Proceed to [next section](https://github.com/nxvl/secure-coding-with-python/tree/3.1-weak-password-storage/fix)** -======= -## Chapter 2: SQL Injection -### Fix part 3 -An even better approach is to use an ORM, in this case we set up SQLAlchemy, by using the standard methods the ORM will do the sanitization so we don't need to worry about it. - -**Note**: Most ORMs in some special use cases can still allow SQL Injections to happen, if you are using non-standard methods, review the ORMs security guidelines and test your application. - -## Description -Welcome to the Secure coding with python course. In this repository you will find a series of branches for each step of the development of a sample marketplace application. In such a development, we will be making security mistakes and introducing vulnerabilities, we will add tests for them and finally fixing them. - -The branches will have the following naming scheme for easier navigation: {Chapter number}-{Chapter Name}/{code|test|fix}. I encourage you to follow the chapters in order, but you can also skip to the specific one you wish to review. - -For this course we will be using Python3, Flask and PostgreSQL. - -**Proceed to [next section](https://github.com/nxvl/secure-coding-with-python/tree/3.1-weak-password-storage/code)** ->>>>>>> 3ff4b7e25b966ef82e6bc6fbfbaae12608f4cb68 ## Index ### 1. Vulnerable Components