From 38449db2ddece504ed39da5b0a5a538a73dbd1e6 Mon Sep 17 00:00:00 2001 From: Nicolas Valcarcel Date: Sat, 6 Jul 2019 11:32:14 -0500 Subject: [PATCH 1/2] Add a vulnerable listing creation form and a listing index page to show the results --- README.md | 61 +++++----------------- marketplace/__init__.py | 24 +++++++-- marketplace/db.py | 37 +++++++++++++ marketplace/listings.py | 34 ++++++++++++ marketplace/schema.sql | 8 +++ marketplace/templates/base.html | 9 ++++ marketplace/templates/listings/create.html | 15 ++++++ marketplace/templates/listings/index.html | 21 ++++++++ requirements.txt | 3 +- 9 files changed, 158 insertions(+), 54 deletions(-) create mode 100644 marketplace/db.py create mode 100644 marketplace/listings.py create mode 100644 marketplace/schema.sql create mode 100644 marketplace/templates/base.html create mode 100644 marketplace/templates/listings/create.html create mode 100644 marketplace/templates/listings/index.html diff --git a/README.md b/README.md index ad8f1f4..549db2e 100644 --- a/README.md +++ b/README.md @@ -1,58 +1,21 @@ # Secure Coding with Python. -## Chapter 1: Project Bootstrap +## Chapter 2: SQL Injection ### Requirement -To start with our development, we install Flask, create our requirements.txt with it and create the `marketplace` package, with a minimal Flask app in `__init__.py`. We can run the project with `python -m flask run` to see that it loads correctly. +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. -### Vulnerability -Since we have done some Flask work in the past, we copied over a requirements.txt and installed Flask from it. The version in said file was Flask 0.12. At the date of the development, the latest Flask release is 1.0.3 - -Since Flask 0.12 the following security releases had been issued: -* [0.12.3](https://github.com/pallets/flask/releases/tag/0.12.3): CWE-20: Improper Input Validation on JSON decoding. +### 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. -Given that we used an old version that's vulnerable to all of the above, our application, by definition is vulnerable if we make use of the affected functionallity. - -### Testing -In order to make sure our libraries don't containg any know vulnerabilities, we can use a dependency scanner such as [Safety](https://pyup.io/safety/). +### 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. +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') ``` -(venv) > $ pip install safety -(venv) > $ safety check -r requirements.txt --full-report -╒══════════════════════════════════════════════════════════════════════════════╕ -│ │ -│ /$$$$$$ /$$ │ -│ /$$__ $$ | $$ │ -│ /$$$$$$$ /$$$$$$ | $$ \__//$$$$$$ /$$$$$$ /$$ /$$ │ -│ /$$_____/ |____ $$| $$$$ /$$__ $$|_ $$_/ | $$ | $$ │ -│ | $$$$$$ /$$$$$$$| $$_/ | $$$$$$$$ | $$ | $$ | $$ │ -│ \____ $$ /$$__ $$| $$ | $$_____/ | $$ /$$| $$ | $$ │ -│ /$$$$$$$/| $$$$$$$| $$ | $$$$$$$ | $$$$/| $$$$$$$ │ -│ |_______/ \_______/|__/ \_______/ \___/ \____ $$ │ -│ /$$ | $$ │ -│ | $$$$$$/ │ -│ by pyup.io \______/ │ -│ │ -╞══════════════════════════════════════════════════════════════════════════════╡ -│ REPORT │ -│ checked 1 packages, using default DB │ -╞════════════════════════════╤═══════════╤══════════════════════════╤══════════╡ -│ package │ installed │ affected │ ID │ -╞════════════════════════════╧═══════════╧══════════════════════════╧══════════╡ -│ flask │ 0.12 │ <0.12.3 │ 36388 │ -╞══════════════════════════════════════════════════════════════════════════════╡ -│ flask version Before 0.12.3 contains a CWE-20: Improper Input Validation │ -│ vulnerability in flask that can result in Large amount of memory usage │ -│ possibly leading to denial of service. This attack appear to be exploitable │ -│ via Attacker provides JSON data in incorrect encoding. This vulnerability │ -│ appears to have been fixed in 0.12.3. │ -╘══════════════════════════════════════════════════════════════════════════════╛ -``` -**Note:** The free version of safety updates it's database once a month, so latest vulnerabilities might not show up. For better security a paid API key can be used to get more up-to-date releases information. - -We can start building our CI build script with a simple dependency vulnerabilities check using [Safety](https://pyup.io/safety/) as shown in build.sh - -### Fix -In this case the fix is extremely simple, we just need up upgrade Flask to 1.0.3 in the requirements.txt file. +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. +postgresql ## 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. @@ -61,7 +24,7 @@ The branches will have the following naming scheme for easier navigation: {Chapt For this course we will be using Python3, Flask and PostgreSQL. -**Proceed to [next section](https://github.com/nxvl/secure-coding-with-python/tree/2.1-sql-injection/code)** +**Proceed to [next section](https://github.com/nxvl/secure-coding-with-python/tree/2.1-sql-injection/test)** ## Index ### 1. Vulnerable Components diff --git a/marketplace/__init__.py b/marketplace/__init__.py index 1b4f4af..a23df9c 100644 --- a/marketplace/__init__.py +++ b/marketplace/__init__.py @@ -1,7 +1,23 @@ +import os + from flask import Flask -app = Flask(__name__) +def create_app(test_config=None): + app = Flask(__name__, instance_relative_config=True) + app.config.from_mapping( + SECRET_KEY='dev', + DATABASE='marketplace', + ) + + try: + os.makedirs(app.instance_path) + except OSError: + pass + + from . import db + db.init_app(app) + + from . import listings + app.register_blueprint(listings.bp) -@app.route('/') -def hello(): - return 'Hello, World!' \ No newline at end of file + return app \ No newline at end of file diff --git a/marketplace/db.py b/marketplace/db.py new file mode 100644 index 0000000..b60ee14 --- /dev/null +++ b/marketplace/db.py @@ -0,0 +1,37 @@ +import psycopg2 + +import click +from flask import current_app, g +from flask.cli import with_appcontext + +def get_db(): + if 'db' not in g: + g.db = psycopg2.connect(dbname=current_app.config['DATABASE']) + + return g.db + +def close_db(e=None): + db = g.pop('db', None) + + if db is not None: + db.close() + +def init_db(): + db = get_db() + cur = db.cursor() + + with current_app.open_resource('schema.sql') as f: + cur.execute(f.read().decode('utf8')) + db.commit() + + +@click.command('init-db') +@with_appcontext +def init_db_command(): + """Clear the existing data and create new tables.""" + init_db() + click.echo('Initialized the database.') + +def init_app(app): + app.teardown_appcontext(close_db) + app.cli.add_command(init_db_command) \ No newline at end of file diff --git a/marketplace/listings.py b/marketplace/listings.py new file mode 100644 index 0000000..3f9dbcb --- /dev/null +++ b/marketplace/listings.py @@ -0,0 +1,34 @@ +import sys + +from flask import Blueprint, request, redirect, render_template, url_for + +from marketplace.db import get_db + +bp = Blueprint('listings', __name__, url_prefix='/listings') + +@bp.route('/') +def index(): + cur = get_db().cursor() + cur.execute( + 'SELECT id, title, description' + ' FROM listings' + ) + listings = cur.fetchall() + return render_template('listings/index.html', listings=listings) + +@bp.route('/create', methods=('GET', 'POST')) +def register(): + if request.method == 'POST': + title = request.form['title'] + description = request.form['description'] + db = get_db() + cur = db.cursor() + + sql = "INSERT INTO listings (title, description) VALUES ('%s', '%s')" % (title, description) + print(sql, file=sys.stdout) + cur.execute(sql) + db.commit() + return redirect(url_for('listings.index')) + + return render_template('listings/create.html') + diff --git a/marketplace/schema.sql b/marketplace/schema.sql new file mode 100644 index 0000000..f9d122d --- /dev/null +++ b/marketplace/schema.sql @@ -0,0 +1,8 @@ +DROP TABLE IF EXISTS listings; + +CREATE TABLE listings( + id SERIAL NOT NULL, + title VARCHAR(128) NOT NULL, + description VARCHAR(500) NOT NULL, + PRIMARY KEY (id) +); \ No newline at end of file diff --git a/marketplace/templates/base.html b/marketplace/templates/base.html new file mode 100644 index 0000000..c6e57df --- /dev/null +++ b/marketplace/templates/base.html @@ -0,0 +1,9 @@ + +{% block title %}{% endblock %} - Secure Coding Python + +
+
+ {% block header %}{% endblock %} +
+ {% block content %}{% endblock %} +
\ No newline at end of file diff --git a/marketplace/templates/listings/create.html b/marketplace/templates/listings/create.html new file mode 100644 index 0000000..67c34d3 --- /dev/null +++ b/marketplace/templates/listings/create.html @@ -0,0 +1,15 @@ +{% extends 'base.html' %} + +{% block header %} +

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

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

{% block title %}Listings{% endblock %}

+{% endblock %} + +{% block content %} + {% for listing in listings %} +
+
+
+

{{ listing[1] }}

+
+
+

{{ listing[2] }}

+
+ {% if not loop.last %} +
+ {% endif %} + {% endfor %} +{% endblock %} \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index e05b516..f147c86 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1,2 @@ -Flask==1.0.3 \ No newline at end of file +Flask==1.0.3 +psycopg2==2.8.3 From 31ac36a819ae3326580ef7da4b726b154b849c34 Mon Sep 17 00:00:00 2001 From: Nicolas Valcarcel Date: Sat, 6 Jul 2019 11:34:58 -0500 Subject: [PATCH 2/2] typo --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 549db2e..70dd123 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,6 @@ We can for example get the postgresql version or any other SQL function result, 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. -postgresql ## 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.