From a05ade56f0c25f38c221705960a5d513a7410cb8 Mon Sep 17 00:00:00 2001 From: Nicolas Valcarcel Date: Sat, 6 Jul 2019 18:57:20 -0500 Subject: [PATCH 1/4] add a new test case to our fuzzer, run bandit --- README.md | 36 ++++++++++++++++++++++++++++++++++++ tests/helpers/sqlifuzzer.py | 2 +- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 38cd91e..5f50d84 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,42 @@ INSERT INTO listings (title, description) VALUES (E'injection\\', (select versio ``` 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. + ## 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. diff --git a/tests/helpers/sqlifuzzer.py b/tests/helpers/sqlifuzzer.py index 7d8629a..60759af 100644 --- a/tests/helpers/sqlifuzzer.py +++ b/tests/helpers/sqlifuzzer.py @@ -4,7 +4,7 @@ def sqli_fuzzer(client, url, params): fail = False - injections = ["'"] + injections = ["'", "\\'"] for injection in injections: for param in params: data = {k: 'foo' for k in params} From 10c5f16155a47c41f65a93319e8a018924d900c4 Mon Sep 17 00:00:00 2001 From: Nicolas Valcarcel Date: Sat, 6 Jul 2019 18:58:43 -0500 Subject: [PATCH 2/4] update next section link --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5f50d84..a0a7fb3 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,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.2-sql-injection/test2)** +**Proceed to [next section](https://github.com/nxvl/secure-coding-with-python/tree/2.2-sql-injection/fix2)** ## Index ### 1. Vulnerable Components From 0b4532a814d01c8c2a35fc5c05a7d61d119967b8 Mon Sep 17 00:00:00 2001 From: Nicolas Valcarcel Date: Sat, 3 Aug 2019 15:33:24 -0500 Subject: [PATCH 3/4] change next section link --- README.md | 92 +------------------------------------------------------ 1 file changed, 1 insertion(+), 91 deletions(-) diff --git a/README.md b/README.md index a0a7fb3..bc11749 100644 --- a/README.md +++ b/README.md @@ -1,96 +1,6 @@ # Secure Coding with Python. ## Chapter 2: SQL Injection -### 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. - -### 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. - -### 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') -``` -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/). @@ -134,7 +44,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.2-sql-injection/fix2)** +**Proceed to [next section](https://github.com/nxvl/secure-coding-with-python/tree/2.2-sql-injection/fix)** ## Index ### 1. Vulnerable Components From e290483ac46c7b87e6df1c946102776c740886e3 Mon Sep 17 00:00:00 2001 From: Nicolas Valcarcel Date: Sat, 3 Aug 2019 15:43:46 -0500 Subject: [PATCH 4/4] clean README --- README.md | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/README.md b/README.md index 40dbe28..4c96cc4 100644 --- a/README.md +++ b/README.md @@ -1,21 +1,9 @@ # Secure Coding with Python. ## Chapter 2: SQL Injection -<<<<<<< HEAD ### 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/). -======= -### 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: ->>>>>>> 92ad6fd2f124e1ba42cdec5246bd229775808ed0 ```text (venv) > $ bandit marketplace/**/*.py Test results: @@ -56,11 +44,7 @@ The branches will have the following naming scheme for easier navigation: {Chapt For this course we will be using Python3, Flask and PostgreSQL. -<<<<<<< HEAD **Proceed to [next section](https://github.com/nxvl/secure-coding-with-python/tree/2.2-sql-injection/fix)** -======= -**Proceed to [next section](https://github.com/nxvl/secure-coding-with-python/tree/2.2-sql-injection/test)** ->>>>>>> 92ad6fd2f124e1ba42cdec5246bd229775808ed0 ## Index ### 1. Vulnerable Components