Skip to content

Commit

Permalink
Merge pull request #28 from tonybaloney/sql_injection
Browse files Browse the repository at this point in the history
Sql Injection
  • Loading branch information
tonybaloney authored Jan 29, 2020
2 parents e19ce3b + e84a2d8 commit ef74eb1
Show file tree
Hide file tree
Showing 9 changed files with 317 additions and 7 deletions.
3 changes: 2 additions & 1 deletion HISTORY.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Release History

## 1.1.1
## 1.2.0

* Added SQL injection with Python formatting check [SQL100](doc/checks/SQL100.md)
* Support for PyCharm 2020.1

## 1.1.0
Expand Down
5 changes: 3 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ plugins {
}

group 'org.tonybaloney.security'
version '1.1.1'
version '1.2.0'

repositories {
mavenCentral()
Expand Down Expand Up @@ -36,9 +36,10 @@ intellij {

patchPluginXml {
changeNotes """
<h2>1.1.1</h2>
<h2>1.2.0</h2>
<h3>New Features</h3>
<ul>
<li>Checks for potential SQL injection with Python formatting expressions</li>
<li>Support for PyCharm 2020.1</li>
</ul>
"""
Expand Down
1 change: 1 addition & 0 deletions doc/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@
* [JJ100](checks/JJ100.md) Use of Jinja2 without autoescaped input
* [EX100](checks/EX100.md) Use of builtin `exec()` function
* [MK100](checks/MK100.md) Use of Mako template without escaped input
* [SQL100](checks/SQL100.md) Possible SQL injection with Python string formatting
48 changes: 48 additions & 0 deletions doc/checks/SQL100.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# SQL100

Looks for SQL injection by Python string formatting methods. Includes:

- Use of "f-string"
- Use for string.format()
- Use of `%` formatting

Will look for formatted string literals that start with:

- `INSERT INTO `
- `DELETE FROM`
- `ALTER TABLE `
- `DROP DATABASE `
- `CREATE DATABASE `

It will also look for strings that start with `SELECT ` and contain ` FROM `, as well as strings that start with ` UPDATE ` and contain ` SET `.

Check is case-insensitive.

This check does not verify that the input is sanitized.

## Examples

Each of the following expressions would trigger a warning for this check:

```python
id = get_id() # Could be a SQLi response..

query1 = f"SELECT * FROM users WHERE id = {0}"

query2 = "SELECT * FROM users WHERE id = {0}" % id

query3 = "SELECT * FROM users WHERE id = {0}".format(id)

query4 = f"UPDATE users SET is_admin = 1 WHERE id = {0}"

query5 = f"DELETE FROM users WHERE id = {0}"

query6 = f"INSERT INTO users (id) VALUES ( id = {0} )"

query7 = f"SELECT * FROM users WHERE id = {0}"

```

## Fixes

Apply input validation and escaping.
1 change: 1 addition & 0 deletions src/main/java/security/Checks.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ object Checks {
val JinjaAutoinspectCheck = CheckType("JJ100", "Jinja does not inspect or sanitize input by default, leaving rendered templates open to XSS. Use autoinspect=True.")
val BuiltinExecCheck = CheckType("EX100", "Use of builtin exec function for dynamic input is insecure and can leave your application open to arbitrary code execution.")
val MakoTemplateFilterCheck = CheckType("MK100", "Mako does not inspect or sanitize input by default, leaving rendered templates open to XSS. Use default_filters=['h'].")
val SqlInjectionCheck = CheckType("SQL100", "Possible SQL injection within String format")

class CheckType(var Code: String, var Message: String) {
override fun toString(): String {
Expand Down
66 changes: 66 additions & 0 deletions src/main/java/security/validators/SqlInjectionInspection.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
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.*
import security.Checks

class SqlInjectionInspection : PyInspection() {
val check = Checks.SqlInjectionCheck;

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) {
// Double-word SQL commands (high-certainty)
val certainlySqlStartingStrings = arrayOf("INSERT INTO ", "DELETE FROM", "ALTER TABLE ", "DROP DATABASE ", "CREATE DATABASE ")
// Double-word SQL commands (low-certainty)
val possiblySqlCommandPairs = mapOf<String, String>("SELECT " to " FROM ", "UPDATE " to " SET ")

fun looksLikeSql(str: String) : Boolean {
// Quickly respond to double-worded SQL statements
if (certainlySqlStartingStrings.any { str.toUpperCase().startsWith(it) }) return true

// SELECT must contain FROM, and UPDATE must contain SET
possiblySqlCommandPairs.forEach { pair ->
if (str.toUpperCase().startsWith(pair.key) && str.toUpperCase().contains(pair.value))
return true
}
return false
}

override fun visitPyFormattedStringElement(node: PyFormattedStringElement?) {
// F-string
if (node == null) return
if (!looksLikeSql(node.content)) return
holder?.registerProblem(node, Checks.SqlInjectionCheck.getDescription())
}

override fun visitPyStringLiteralExpression(node: PyStringLiteralExpression?) {
if (node == null) return
if (!looksLikeSql(node.stringValue)) return

// .Format() string
if (node.parent is PyReferenceExpression){
if ((node.parent as PyReferenceExpression).name != "format") return
if (node.parent.parent == null) return
if (node.parent.parent !is PyCallExpression) return
holder?.registerProblem(node, Checks.SqlInjectionCheck.getDescription())
}

// % format string
if (node.parent is PyBinaryExpression) {
if ((node.parent as PyBinaryExpression).operator.toString() != "Py:PERC") return
holder?.registerProblem(node, Checks.SqlInjectionCheck.getDescription())
}
}
}
}
2 changes: 2 additions & 0 deletions src/main/resources/META-INF/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
<localInspection language="Python" enabledByDefault="true" groupName="Python Security" hasStaticDescription="true" displayName="PW101: Hardcoded passwords and secrets" shortName="HardcodedPasswordInspection" implementationClass="security.validators.HardcodedPasswordInspection" />
<localInspection language="Python" enabledByDefault="true" groupName="Python Security" hasStaticDescription="true" displayName="EX100: Use of builtin exec() function" shortName="BuiltinExecInspection" implementationClass="security.validators.BuiltinExecInspection" />
<localInspection language="Python" enabledByDefault="true" groupName="Python Security" hasStaticDescription="true" displayName="MK100: Use of unsanitized Mako Templates" shortName="MakoTemplateInspection" implementationClass="security.validators.MakoTemplateInspection" />
<localInspection language="Python" enabledByDefault="true" groupName="Python Security" hasStaticDescription="true" displayName="SQL100: Possible SQL injection" shortName="SqlInjectionInspection" implementationClass="security.validators.SqlInjectionInspection" />

</extensions>
<extensions defaultExtensionNs="Pythonid">
</extensions>
Expand Down
49 changes: 45 additions & 4 deletions src/test/java/security/SecurityTestTask.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ import com.intellij.testFramework.fixtures.BasePlatformTestCase
import com.jetbrains.python.PythonFileType
import com.jetbrains.python.inspections.PyInspection
import com.jetbrains.python.inspections.PyInspectionVisitor
import com.jetbrains.python.psi.PyAssignmentStatement
import com.jetbrains.python.psi.PyBinaryExpression
import com.jetbrains.python.psi.PyCallExpression
import com.jetbrains.python.psi.PyExpression
import com.jetbrains.python.psi.*
import com.nhaarman.mockitokotlin2.*
import org.jetbrains.annotations.NotNull
import org.mockito.Mockito
Expand Down Expand Up @@ -85,4 +82,48 @@ open class SecurityTestTask: BasePlatformTestCase() {
Mockito.verify(mockLocalSession, Mockito.times(1)).file
}
}

inline fun <inspector: PyInspection>testStringLiteralExpression(code: String, times: Int = 1, check: Checks.CheckType, filename: String = "test.py", instance: inspector){
ApplicationManager.getApplication().runReadAction {
val mockHolder = mock<ProblemsHolder> {
on { registerProblem(any<PsiElement>(), eq(check.getDescription()), anyVararg<LocalQuickFix>()) } doAnswer {}
}
val testFile = this.createLightFile(filename, PythonFileType.INSTANCE.language, code);
val mockLocalSession = mock<LocalInspectionToolSession> {
on { file } doReturn (testFile)
}
assertNotNull(testFile)
val testVisitor = instance.buildVisitor(mockHolder, true, mockLocalSession) as PyInspectionVisitor

val expr: @NotNull MutableCollection<PyStringLiteralExpression> = PsiTreeUtil.findChildrenOfType(testFile, PyStringLiteralExpression::class.java)
assertNotNull(expr)
expr.forEach { e ->
testVisitor.visitPyStringLiteralExpression(e)
}
Mockito.verify(mockHolder, Mockito.times(times)).registerProblem(any<PsiElement>(), eq(check.getDescription()), anyVararg<LocalQuickFix>())
Mockito.verify(mockLocalSession, Mockito.times(1)).file
}
}

inline fun <inspector: PyInspection>testFormattedStringElement(code: String, times: Int = 1, check: Checks.CheckType, filename: String = "test.py", instance: inspector){
ApplicationManager.getApplication().runReadAction {
val mockHolder = mock<ProblemsHolder> {
on { registerProblem(any<PsiElement>(), eq(check.getDescription()), anyVararg<LocalQuickFix>()) } doAnswer {}
}
val testFile = this.createLightFile(filename, PythonFileType.INSTANCE.language, code);
val mockLocalSession = mock<LocalInspectionToolSession> {
on { file } doReturn (testFile)
}
assertNotNull(testFile)
val testVisitor = instance.buildVisitor(mockHolder, true, mockLocalSession) as PyInspectionVisitor

val expr: @NotNull MutableCollection<PyFormattedStringElement> = PsiTreeUtil.findChildrenOfType(testFile, PyFormattedStringElement::class.java)
assertNotNull(expr)
expr.forEach { e ->
testVisitor.visitPyFormattedStringElement(e)
}
Mockito.verify(mockHolder, Mockito.times(times)).registerProblem(any<PsiElement>(), eq(check.getDescription()), anyVararg<LocalQuickFix>())
Mockito.verify(mockLocalSession, Mockito.times(1)).file
}
}
}
149 changes: 149 additions & 0 deletions src/test/java/security/validators/SqlInjectionInspectionTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
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 SqlInjectionInspectionTest: SecurityTestTask() {
@BeforeAll
override fun setUp() {
super.setUp()
}

@AfterAll
override fun tearDown(){
super.tearDown()
}

@Test
fun `verify description is not empty`(){
assertFalse(SqlInjectionInspection().staticDescription.isNullOrEmpty())
}

@Test
fun `test format string string select`(){
var code = """
query = "SELECT * FROM users WHERE id = {0}".format(id)
""".trimIndent()
testStringLiteralExpression(code, 1, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string not select`(){
var code = """
query = "SELECT a banana {0}".format(id)
""".trimIndent()
testStringLiteralExpression(code, 0, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string update`(){
var code = """
query = "UPDATE users SET id = {0} WHERE x=1".format(id)
""".trimIndent()
testStringLiteralExpression(code, 1, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string string not update`(){
var code = """
query = "UPDATE a banana {0}".format(id)
""".trimIndent()
testStringLiteralExpression(code, 0, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}


@Test
fun `test insert into format function`(){
var code = """
query = "INSERT INTO users (id) VALUES ( id = {0} )".format(id)
""".trimIndent()
testStringLiteralExpression(code, 1, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string string select perc format`(){
var code = """
query = "SELECT * FROM users WHERE id = {0}" % id
""".trimIndent()
testStringLiteralExpression(code, 1, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string not select perc format`(){
var code = """
query = "SELECT a banana {0}" % id
""".trimIndent()
testStringLiteralExpression(code, 0, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string update perc format`(){
var code = """
query = "UPDATE users SET id = {0} WHERE x=1" % id
""".trimIndent()
testStringLiteralExpression(code, 1, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string string not update perc format`(){
var code = """
query = "UPDATE a banana {0}" % id
""".trimIndent()
testStringLiteralExpression(code, 0, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}


@Test
fun `test insert into format function perc format`(){
var code = """
query = "INSERT INTO users (id) VALUES ( id = {0} )" % id
""".trimIndent()
testStringLiteralExpression(code, 1, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string string select fstring format`(){
var code = """
query = f"SELECT * FROM users WHERE id = {0}"
""".trimIndent()
testFormattedStringElement(code, 1, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string not select fstring format`(){
var code = """
query = f"SELECT a banana {0}"
""".trimIndent()
testFormattedStringElement(code, 0, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string update fstring format`(){
var code = """
query = f"UPDATE users SET id = {0} WHERE x=1"
""".trimIndent()
testFormattedStringElement(code, 1, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}

@Test
fun `test format string string not update fstring format`(){
var code = """
query = f"UPDATE a banana {0}"
""".trimIndent()
testFormattedStringElement(code, 0, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}


@Test
fun `test insert into format function fstring format`(){
var code = """
query = f"INSERT INTO users (id) VALUES ( id = {0} )"
""".trimIndent()
testFormattedStringElement(code, 1, Checks.SqlInjectionCheck, "test.py", SqlInjectionInspection())
}
}

0 comments on commit ef74eb1

Please sign in to comment.