-
-
Notifications
You must be signed in to change notification settings - Fork 21
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #30 from tonybaloney/django_sql
Django SQL Injection techniques
- Loading branch information
Showing
12 changed files
with
315 additions
and
10 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| # DJG101 | ||
|
|
||
| This check looks for ways that Django SQL Injection protection is being bypassed, by using quoted parameters. | ||
|
|
||
| The check looks at the following use cases: | ||
|
|
||
| * Use of `RawSQL()` constructor directly | ||
| * Use of `cursor.execute()` | ||
| * Use of `raw()` on a `Manager` instance | ||
|
|
||
| Whilst the methods support parametrized queries, if the `%s` value is quoted with single-quotes, the value is still vulnerable to SQL injection. | ||
|
|
||
| ## Example | ||
|
|
||
| The first example is using the RawSQL constructor directly and annotating a query set: | ||
|
|
||
| ```python | ||
| from django.db.models.expressions import RawSQL | ||
|
|
||
| qs.annotate(val=RawSQL("select col from sometable where othercol = '%s'", (someparam,))) # this is bad! | ||
| ``` | ||
|
|
||
| Another example is using the `raw()` method on a manager to filter results, exposing SQL injection: | ||
|
|
||
| ```python | ||
| from django import things | ||
| from .models import User | ||
|
|
||
| def my_view(self): | ||
| User.objects.raw("SELECT * FROM myapp_person WHERE last_name = '%s'", [lname]) # this is also bad! | ||
| ``` | ||
|
|
||
| Cursors can also be exploited using the same technique: | ||
|
|
||
| ```python | ||
| from django.db import connection | ||
|
|
||
| def my_custom_sql(self): | ||
| with connection.cursor() as cursor: | ||
| cursor.execute("UPDATE bar SET foo = 1 WHERE baz = %s", [self.baz]) | ||
| cursor.execute("SELECT foo FROM bar WHERE baz = '%s'", [self.baz]) | ||
| row = cursor.fetchone() | ||
| return row | ||
| ``` | ||
|
|
||
| ## Fixes | ||
|
|
||
| Remove the quotations from the string values: | ||
|
|
||
| ```python | ||
| ("UPDATE bar SET foo = 1 WHERE baz = %s", [self.baz]) # good | ||
| ("UPDATE bar SET foo = 1 WHERE baz = '%s'", [self.baz]) # bad! | ||
| ``` | ||
|
|
||
| ## See Also | ||
|
|
||
| * [Official Documentation](https://docs.djangoproject.com/en/3.0/ref/models/expressions/#django.db.models.expressions.RawSQL) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package security.helpers | ||
|
|
||
| import com.jetbrains.python.psi.PyFile | ||
|
|
||
| object ImportValidators { | ||
| fun hasImportedNamespace(file: PyFile, match: String) : Boolean { | ||
| val imports = file.importBlock ?: return false | ||
| if (imports.isEmpty()) return false | ||
| for (imp in imports){ | ||
| if (imp.fullyQualifiedObjectNames.isEmpty()) continue | ||
| if (imp.fullyQualifiedObjectNames.first() == match) return true | ||
| if (imp.fullyQualifiedObjectNames.first().startsWith("$match.")) return true | ||
| } | ||
| return false | ||
| } | ||
| } |
51 changes: 51 additions & 0 deletions
51
src/main/java/security/validators/DjangoRawSqlInspection.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package security.validators | ||
|
|
||
| import com.intellij.codeInspection.LocalInspectionToolSession | ||
| import com.intellij.codeInspection.ProblemsHolder | ||
| import com.intellij.psi.PsiElementVisitor | ||
| import com.jetbrains.python.inspections.PyInspection | ||
| import com.jetbrains.python.inspections.PyInspectionVisitor | ||
| import com.jetbrains.python.psi.PyCallExpression | ||
| import com.jetbrains.python.psi.PyFile | ||
| import com.jetbrains.python.psi.PyStringLiteralExpression | ||
| import security.Checks | ||
| import security.helpers.ImportValidators.hasImportedNamespace | ||
|
|
||
| class DjangoRawSqlInspection : PyInspection() { | ||
| val check = Checks.DjangoClickjackMiddlewareCheck; | ||
|
|
||
| override fun getStaticDescription(): String? { | ||
| return check.getDescription() | ||
| } | ||
|
|
||
| override fun buildVisitor(holder: ProblemsHolder, | ||
| isOnTheFly: Boolean, | ||
| session: LocalInspectionToolSession): PsiElementVisitor = Visitor(holder, session) | ||
|
|
||
| private class Visitor(holder: ProblemsHolder, session: LocalInspectionToolSession) : PyInspectionVisitor(holder, session) { | ||
| val methodNames = arrayOf("RawSQL", "raw", "execute") | ||
| override fun visitPyCallExpression(node: PyCallExpression?) { | ||
| if (node == null) return | ||
| val calleeName = node.callee?.name ?: return | ||
| if (!listOf(*methodNames).contains(calleeName)) return | ||
|
|
||
| if (node.containingFile !is PyFile) return | ||
| if (!hasImportedNamespace(node.containingFile as PyFile, "django")) return | ||
|
|
||
| val sqlStatement = node.arguments.first() ?: return | ||
| if (sqlStatement !is PyStringLiteralExpression) return | ||
| val param = Regex("%s") | ||
| val paramMatches = param.findAll(sqlStatement.stringValue) | ||
| for (match in paramMatches){ | ||
| try { | ||
| if (sqlStatement.stringValue.substring(match.range.start - 1, match.range.start) != "'") return | ||
| if (sqlStatement.stringValue.substring(match.range.last + 1, match.range.last + 2) != "'") return | ||
| } catch (oobe: StringIndexOutOfBoundsException){ | ||
| // End or beginning of string, so this SQL injection technique wouldn't be possible. | ||
| return | ||
| } | ||
| holder?.registerProblem(node, Checks.DjangoRawSqlCheck.getDescription()) | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package security.helpers | ||
|
|
||
| import com.intellij.openapi.application.ApplicationManager | ||
| import com.jetbrains.python.PythonFileType | ||
| import com.jetbrains.python.psi.PyFile | ||
| import org.junit.jupiter.api.AfterAll | ||
| import org.junit.jupiter.api.BeforeAll | ||
| import org.junit.jupiter.api.Test | ||
| import org.junit.jupiter.api.TestInstance | ||
| import security.SecurityTestTask | ||
| import security.helpers.ImportValidators.hasImportedNamespace | ||
|
|
||
| @TestInstance(TestInstance.Lifecycle.PER_CLASS) | ||
| class ImportValidatorsTest: SecurityTestTask() { | ||
|
|
||
| @BeforeAll | ||
| override fun setUp() { | ||
| super.setUp() | ||
| } | ||
|
|
||
| @AfterAll | ||
| override fun tearDown(){ | ||
| super.tearDown() | ||
| } | ||
|
|
||
| @Test | ||
| fun `test empty file`(){ | ||
| var code = """ | ||
| """.trimIndent() | ||
| assertFalse(testHasImport(code, "django")) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test file with no imports`(){ | ||
| var code = """ | ||
| x = 1 | ||
| """.trimIndent() | ||
| assertFalse(testHasImport(code, "django")) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test simple import`(){ | ||
| var code = """ | ||
| import django | ||
| """.trimIndent() | ||
| assertTrue(testHasImport(code, "django")) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test from import`(){ | ||
| var code = """ | ||
| from django.db import DbConnection | ||
| """.trimIndent() | ||
| assertTrue(testHasImport(code, "django")) | ||
| } | ||
|
|
||
| private fun testHasImport(code: String, importName: String): Boolean{ | ||
| var hasImport: Boolean = false | ||
| ApplicationManager.getApplication().runReadAction { | ||
| val testFile = this.createLightFile("test.py", PythonFileType.INSTANCE.language, code); | ||
| assertNotNull(testFile) | ||
| hasImport = hasImportedNamespace(testFile as PyFile, importName) | ||
| } | ||
| return hasImport | ||
| } | ||
| } |
108 changes: 108 additions & 0 deletions
108
src/test/java/security/validators/DjangoRawSqlInspectionTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| package security.validators | ||
|
|
||
| import org.junit.jupiter.api.AfterAll | ||
| import org.junit.jupiter.api.BeforeAll | ||
| import org.junit.jupiter.api.Test | ||
| import org.junit.jupiter.api.TestInstance | ||
| import security.Checks | ||
| import security.SecurityTestTask | ||
|
|
||
| @TestInstance(TestInstance.Lifecycle.PER_CLASS) | ||
| class DjangoRawSqlInspectionTest: SecurityTestTask() { | ||
| @BeforeAll | ||
| override fun setUp() { | ||
| super.setUp() | ||
| } | ||
|
|
||
| @AfterAll | ||
| override fun tearDown(){ | ||
| super.tearDown() | ||
| } | ||
|
|
||
| @Test | ||
| fun `verify description is not empty`(){ | ||
| assertFalse(DjangoRawSqlInspection().staticDescription.isNullOrEmpty()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test quoted string`(){ | ||
| var code = """ | ||
| import django.db.models.expressions | ||
| x = "injectable string" | ||
| django.db.models.expressions.RawSQL("SELECT * FROM foo WHERE ID = '%s'", (x,)) | ||
| """.trimIndent() | ||
| testCodeCallExpression(code, 1, Checks.DjangoRawSqlCheck, "test.py", DjangoRawSqlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test format non quoted string`(){ | ||
| var code = """ | ||
| import django.db.models.expressions | ||
| x = "injectable string" | ||
| django.db.models.expressions.RawSQL("SELECT * FROM foo WHERE ID = %s", (x,)) | ||
| """.trimIndent() | ||
| testCodeCallExpression(code, 0, Checks.DjangoRawSqlCheck, "test.py", DjangoRawSqlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test some other rawsql method`(){ | ||
| var code = """ | ||
| RawSQL("SELECT * FROM foo WHERE ID = '%s'", (x,)) | ||
| """.trimIndent() | ||
| testCodeCallExpression(code, 0, Checks.DjangoRawSqlCheck, "test.py", DjangoRawSqlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test cursor execute with no quotes`(){ | ||
| var code = """ | ||
| from django.db import connection | ||
| def my_custom_sql(self): | ||
| with connection.cursor() as cursor: | ||
| cursor.execute("UPDATE bar SET foo = 1 WHERE baz = %s", [self.baz]) | ||
| cursor.execute("SELECT foo FROM bar WHERE baz = %s", [self.baz]) | ||
| row = cursor.fetchone() | ||
| return row | ||
| """.trimIndent() | ||
| testCodeCallExpression(code, 0, Checks.DjangoRawSqlCheck, "test.py", DjangoRawSqlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test cursor execute with quotes`(){ | ||
| var code = """ | ||
| from django.db import connection | ||
| def my_custom_sql(self): | ||
| with connection.cursor() as cursor: | ||
| cursor.execute("UPDATE bar SET foo = 1 WHERE baz = %s", [self.baz]) | ||
| cursor.execute("SELECT foo FROM bar WHERE baz = '%s'", [self.baz]) | ||
| row = cursor.fetchone() | ||
| return row | ||
| """.trimIndent() | ||
| testCodeCallExpression(code, 1, Checks.DjangoRawSqlCheck, "test.py", DjangoRawSqlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test model raw with quotes`(){ | ||
| var code = """ | ||
| from django.db import connection | ||
| from .models import User | ||
| def my_view(self): | ||
| User.objects.raw("SELECT * FROM myapp_person WHERE last_name = '%s'", [lname]) | ||
| """.trimIndent() | ||
| testCodeCallExpression(code, 1, Checks.DjangoRawSqlCheck, "test.py", DjangoRawSqlInspection()) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test model raw with no quotes`(){ | ||
| var code = """ | ||
| from django.db import connection | ||
| from .models import User | ||
| def my_view(self): | ||
| User.objects.raw("SELECT * FROM myapp_person WHERE last_name = %s", [lname]) | ||
| """.trimIndent() | ||
| testCodeCallExpression(code, 0, Checks.DjangoRawSqlCheck, "test.py", DjangoRawSqlInspection()) | ||
| } | ||
| } |