fix: improve gosec sqli (#187)

Bearer · Dec 15, 2023 · 889f769 · 889f769
1 parent 54a9cc0
commit 889f769
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 33 deletions.
diff --git a/rules/go/gosec/sql/concat_sqli.yml b/rules/go/gosec/sql/concat_sqli.yml
@@ -5,11 +5,14 @@ patterns:
       $<DB>.Query($<INPUT>)
     filters:
       - variable: INPUT
-        detection: go_gosec_sql_concat_sqli_sanitized_input
+        detection: go_gosec_sql_concat_sqli_unsanitized_input
       - not:
           variable: INPUT
           detection: go_gosec_sql_concat_sqli_input_sprintf_sanitizer
       - either:
+          - variable: DB
+            detection: go_gosec_sql_concat_sqli_sql_db_init
+            scope: cursor
           - variable: DB
             detection: go_gosec_sql_concat_sqli_sql_open
             scope: cursor
@@ -20,7 +23,7 @@ patterns:
       $<DB>.QueryContext($<...>$<INPUT>)
     filters:
       - variable: INPUT
-        detection: go_gosec_sql_concat_sqli_sanitized_input
+        detection: go_gosec_sql_concat_sqli_unsanitized_input
       - either:
           - variable: DB
             detection: go_gosec_sql_concat_sqli_sql_open
@@ -38,14 +41,26 @@ auxiliary:
   - id: go_gosec_sql_concat_sqli_input_sanitizer
     patterns:
       - pattern: $<_>.QuoteIdentifier($<!>$<_>)
-  - id: go_gosec_sql_concat_sqli_sanitized_input
+  - id: go_gosec_sql_concat_sqli_unsanitized_input
     sanitizer: go_gosec_sql_concat_sqli_input_sanitizer
     patterns:
       - pattern: $<INPUT>
         filters:
           - variable: INPUT
             detection: go_shared_lang_dynamic_request_input
             scope: cursor
+  - id: go_gosec_sql_concat_sqli_sql_db_init
+    patterns:
+      - pattern: var $<!>$<_> $<SQL>.DB
+        filters:
+          - variable: SQL
+            detection: go_gosec_sql_concat_sqli_sql_init
+            scope: cursor
+      - pattern: var $<!>$<_> *$<SQL>.DB
+        filters:
+          - variable: SQL
+            detection: go_gosec_sql_concat_sqli_sql_init
+            scope: cursor
   - id: go_gosec_sql_concat_sqli_sql_db_begin
     patterns:
       - pattern: $<SQL>.Begin()

diff --git a/tests/go/gosec/sql/concat_sqli/__snapshots__/test.js.snap b/tests/go/gosec/sql/concat_sqli/__snapshots__/test.js.snap
@@ -11,27 +11,27 @@ exports[`go_gosec_sql_concat_sqli test 1`] = `
       "title": "Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')",
       "description": "## Description\\n\\nSQL Injection represents a severe vulnerability that can culminate in the compromise of data or entire systems. When SQL query strings are crafted dynamically based on user inputs, there's potential for malicious users to manipulate the logic of the SQL statement. Such tampering could provide adversaries unauthorized access to sensitive data or even allow them to execute system-level operations or code.\\n\\n## Remediations\\n\\n✅ Use Parameterized Queries\\n\\nAlways opt for parameterized queries over dynamically generated SQL queries to prevent SQL injection.\\n\\n\`\`\`go\\nrows, err := db.Query(\\"SELECT * FROM users WHERE userName = ?\\", userName)\\nif err != nil {\\n    return nil, err\\n}\\ndefer rows.Close()\\nfor rows.Next() {\\n  // ... process rows\\n}\\n\`\`\`\\n\\n✅ Avoid Direct User Input in Dynamic Queries\\n\\nIf there's an absolute need to formulate dynamic queries, ensure that direct user input is never utilized. Instead, leverage a map or dictionary containing valid values and determine them through a user-provided key.\\n\\nFor instance, certain database drivers do not support parameterized queries for operators like \`>\` or \`<\`. Instead of directly using user-input values, allow users to provide substitutes like \`gt\` for \`>\` and \`lt\` for \`<\`. Subsequently, use these alphabetical inputs to retrieve the actual operators for your query. Implement a similar approach for queries requiring non-parameterizable column or table names.\\n\\n## Resources\\n\\n- [OWASP SQL Injection Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html)\\n",
       "documentation_url": "https://docs.bearer.com/reference/rules/go_gosec_sql_concat_sqli",
-      "line_number": 14,
+      "line_number": 15,
       "full_filename": "/tmp/bearer-scan/main.go",
       "filename": ".",
       "source": {
-        "start": 14,
-        "end": 14,
+        "start": 15,
+        "end": 15,
         "column": {
           "start": 15,
           "end": 71
         }
       },
       "sink": {
-        "start": 14,
-        "end": 14,
+        "start": 15,
+        "end": 15,
         "column": {
           "start": 15,
           "end": 71
         },
         "content": "db.Query(\\"SELECT * FROM foo WHERE name = \\" + os.Args[1])"
       },
-      "parent_line_number": 14,
+      "parent_line_number": 15,
       "snippet": "db.Query(\\"SELECT * FROM foo WHERE name = \\" + os.Args[1])",
       "fingerprint": "ca1a3415f7df18be8dec85c9d74eb890_0",
       "old_fingerprint": "3350ddd230bfd667e45a89b42b9b1b39_0",
@@ -45,27 +45,27 @@ exports[`go_gosec_sql_concat_sqli test 1`] = `
       "title": "Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')",
       "description": "## Description\\n\\nSQL Injection represents a severe vulnerability that can culminate in the compromise of data or entire systems. When SQL query strings are crafted dynamically based on user inputs, there's potential for malicious users to manipulate the logic of the SQL statement. Such tampering could provide adversaries unauthorized access to sensitive data or even allow them to execute system-level operations or code.\\n\\n## Remediations\\n\\n✅ Use Parameterized Queries\\n\\nAlways opt for parameterized queries over dynamically generated SQL queries to prevent SQL injection.\\n\\n\`\`\`go\\nrows, err := db.Query(\\"SELECT * FROM users WHERE userName = ?\\", userName)\\nif err != nil {\\n    return nil, err\\n}\\ndefer rows.Close()\\nfor rows.Next() {\\n  // ... process rows\\n}\\n\`\`\`\\n\\n✅ Avoid Direct User Input in Dynamic Queries\\n\\nIf there's an absolute need to formulate dynamic queries, ensure that direct user input is never utilized. Instead, leverage a map or dictionary containing valid values and determine them through a user-provided key.\\n\\nFor instance, certain database drivers do not support parameterized queries for operators like \`>\` or \`<\`. Instead of directly using user-input values, allow users to provide substitutes like \`gt\` for \`>\` and \`lt\` for \`<\`. Subsequently, use these alphabetical inputs to retrieve the actual operators for your query. Implement a similar approach for queries requiring non-parameterizable column or table names.\\n\\n## Resources\\n\\n- [OWASP SQL Injection Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html)\\n",
       "documentation_url": "https://docs.bearer.com/reference/rules/go_gosec_sql_concat_sqli",
-      "line_number": 26,
+      "line_number": 27,
       "full_filename": "/tmp/bearer-scan/main.go",
       "filename": ".",
       "source": {
-        "start": 26,
-        "end": 26,
+        "start": 27,
+        "end": 27,
         "column": {
           "start": 15,
           "end": 71
         }
       },
       "sink": {
-        "start": 26,
-        "end": 26,
+        "start": 27,
+        "end": 27,
         "column": {
           "start": 15,
           "end": 71
         },
         "content": "db.Query(\\"select * from foo where name = \\" + os.Args[1])"
       },
-      "parent_line_number": 26,
+      "parent_line_number": 27,
       "snippet": "db.Query(\\"select * from foo where name = \\" + os.Args[1])",
       "fingerprint": "ca1a3415f7df18be8dec85c9d74eb890_1",
       "old_fingerprint": "3350ddd230bfd667e45a89b42b9b1b39_1",
@@ -79,27 +79,27 @@ exports[`go_gosec_sql_concat_sqli test 1`] = `
       "title": "Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')",
       "description": "## Description\\n\\nSQL Injection represents a severe vulnerability that can culminate in the compromise of data or entire systems. When SQL query strings are crafted dynamically based on user inputs, there's potential for malicious users to manipulate the logic of the SQL statement. Such tampering could provide adversaries unauthorized access to sensitive data or even allow them to execute system-level operations or code.\\n\\n## Remediations\\n\\n✅ Use Parameterized Queries\\n\\nAlways opt for parameterized queries over dynamically generated SQL queries to prevent SQL injection.\\n\\n\`\`\`go\\nrows, err := db.Query(\\"SELECT * FROM users WHERE userName = ?\\", userName)\\nif err != nil {\\n    return nil, err\\n}\\ndefer rows.Close()\\nfor rows.Next() {\\n  // ... process rows\\n}\\n\`\`\`\\n\\n✅ Avoid Direct User Input in Dynamic Queries\\n\\nIf there's an absolute need to formulate dynamic queries, ensure that direct user input is never utilized. Instead, leverage a map or dictionary containing valid values and determine them through a user-provided key.\\n\\nFor instance, certain database drivers do not support parameterized queries for operators like \`>\` or \`<\`. Instead of directly using user-input values, allow users to provide substitutes like \`gt\` for \`>\` and \`lt\` for \`<\`. Subsequently, use these alphabetical inputs to retrieve the actual operators for your query. Implement a similar approach for queries requiring non-parameterizable column or table names.\\n\\n## Resources\\n\\n- [OWASP SQL Injection Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html)\\n",
       "documentation_url": "https://docs.bearer.com/reference/rules/go_gosec_sql_concat_sqli",
-      "line_number": 38,
+      "line_number": 39,
       "full_filename": "/tmp/bearer-scan/main.go",
       "filename": ".",
       "source": {
-        "start": 38,
-        "end": 38,
+        "start": 39,
+        "end": 39,
         "column": {
           "start": 15,
           "end": 98
         }
       },
       "sink": {
-        "start": 38,
-        "end": 38,
+        "start": 39,
+        "end": 39,
         "column": {
           "start": 15,
           "end": 98
         },
         "content": "db.QueryContext(context.Background(), \\"select * from foo where name = \\"+os.Args[1])"
       },
-      "parent_line_number": 38,
+      "parent_line_number": 39,
       "snippet": "db.QueryContext(context.Background(), \\"select * from foo where name = \\"+os.Args[1])",
       "fingerprint": "ca1a3415f7df18be8dec85c9d74eb890_2",
       "old_fingerprint": "3350ddd230bfd667e45a89b42b9b1b39_2",
@@ -113,27 +113,27 @@ exports[`go_gosec_sql_concat_sqli test 1`] = `
       "title": "Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')",
       "description": "## Description\\n\\nSQL Injection represents a severe vulnerability that can culminate in the compromise of data or entire systems. When SQL query strings are crafted dynamically based on user inputs, there's potential for malicious users to manipulate the logic of the SQL statement. Such tampering could provide adversaries unauthorized access to sensitive data or even allow them to execute system-level operations or code.\\n\\n## Remediations\\n\\n✅ Use Parameterized Queries\\n\\nAlways opt for parameterized queries over dynamically generated SQL queries to prevent SQL injection.\\n\\n\`\`\`go\\nrows, err := db.Query(\\"SELECT * FROM users WHERE userName = ?\\", userName)\\nif err != nil {\\n    return nil, err\\n}\\ndefer rows.Close()\\nfor rows.Next() {\\n  // ... process rows\\n}\\n\`\`\`\\n\\n✅ Avoid Direct User Input in Dynamic Queries\\n\\nIf there's an absolute need to formulate dynamic queries, ensure that direct user input is never utilized. Instead, leverage a map or dictionary containing valid values and determine them through a user-provided key.\\n\\nFor instance, certain database drivers do not support parameterized queries for operators like \`>\` or \`<\`. Instead of directly using user-input values, allow users to provide substitutes like \`gt\` for \`>\` and \`lt\` for \`<\`. Subsequently, use these alphabetical inputs to retrieve the actual operators for your query. Implement a similar approach for queries requiring non-parameterizable column or table names.\\n\\n## Resources\\n\\n- [OWASP SQL Injection Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html)\\n",
       "documentation_url": "https://docs.bearer.com/reference/rules/go_gosec_sql_concat_sqli",
-      "line_number": 55,
+      "line_number": 56,
       "full_filename": "/tmp/bearer-scan/main.go",
       "filename": ".",
       "source": {
-        "start": 55,
-        "end": 55,
+        "start": 56,
+        "end": 56,
         "column": {
           "start": 15,
           "end": 98
         }
       },
       "sink": {
-        "start": 55,
-        "end": 55,
+        "start": 56,
+        "end": 56,
         "column": {
           "start": 15,
           "end": 98
         },
         "content": "tx.QueryContext(context.Background(), \\"select * from foo where name = \\"+os.Args[1])"
       },
-      "parent_line_number": 55,
+      "parent_line_number": 56,
       "snippet": "tx.QueryContext(context.Background(), \\"select * from foo where name = \\"+os.Args[1])",
       "fingerprint": "ca1a3415f7df18be8dec85c9d74eb890_3",
       "old_fingerprint": "3350ddd230bfd667e45a89b42b9b1b39_3",
@@ -147,31 +147,65 @@ exports[`go_gosec_sql_concat_sqli test 1`] = `
       "title": "Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')",
       "description": "## Description\\n\\nSQL Injection represents a severe vulnerability that can culminate in the compromise of data or entire systems. When SQL query strings are crafted dynamically based on user inputs, there's potential for malicious users to manipulate the logic of the SQL statement. Such tampering could provide adversaries unauthorized access to sensitive data or even allow them to execute system-level operations or code.\\n\\n## Remediations\\n\\n✅ Use Parameterized Queries\\n\\nAlways opt for parameterized queries over dynamically generated SQL queries to prevent SQL injection.\\n\\n\`\`\`go\\nrows, err := db.Query(\\"SELECT * FROM users WHERE userName = ?\\", userName)\\nif err != nil {\\n    return nil, err\\n}\\ndefer rows.Close()\\nfor rows.Next() {\\n  // ... process rows\\n}\\n\`\`\`\\n\\n✅ Avoid Direct User Input in Dynamic Queries\\n\\nIf there's an absolute need to formulate dynamic queries, ensure that direct user input is never utilized. Instead, leverage a map or dictionary containing valid values and determine them through a user-provided key.\\n\\nFor instance, certain database drivers do not support parameterized queries for operators like \`>\` or \`<\`. Instead of directly using user-input values, allow users to provide substitutes like \`gt\` for \`>\` and \`lt\` for \`<\`. Subsequently, use these alphabetical inputs to retrieve the actual operators for your query. Implement a similar approach for queries requiring non-parameterizable column or table names.\\n\\n## Resources\\n\\n- [OWASP SQL Injection Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html)\\n",
       "documentation_url": "https://docs.bearer.com/reference/rules/go_gosec_sql_concat_sqli",
-      "line_number": 70,
+      "line_number": 71,
       "full_filename": "/tmp/bearer-scan/main.go",
       "filename": ".",
       "source": {
-        "start": 70,
-        "end": 70,
+        "start": 71,
+        "end": 71,
         "column": {
           "start": 15,
           "end": 75
         }
       },
       "sink": {
-        "start": 70,
-        "end": 70,
+        "start": 71,
+        "end": 71,
         "column": {
           "start": 15,
           "end": 75
         },
         "content": "db.Query(\\"SELECT * FROM foo\\" + \\"WHERE name = \\" + os.Args[1])"
       },
-      "parent_line_number": 70,
+      "parent_line_number": 71,
       "snippet": "db.Query(\\"SELECT * FROM foo\\" + \\"WHERE name = \\" + os.Args[1])",
       "fingerprint": "ca1a3415f7df18be8dec85c9d74eb890_4",
       "old_fingerprint": "3350ddd230bfd667e45a89b42b9b1b39_4",
       "code_extract": "\\trows, err := db.Query(\\"SELECT * FROM foo\\" + \\"WHERE name = \\" + os.Args[1])"
+    },
+    {
+      "cwe_ids": [
+        "89"
+      ],
+      "id": "go_gosec_sql_concat_sqli",
+      "title": "Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')",
+      "description": "## Description\\n\\nSQL Injection represents a severe vulnerability that can culminate in the compromise of data or entire systems. When SQL query strings are crafted dynamically based on user inputs, there's potential for malicious users to manipulate the logic of the SQL statement. Such tampering could provide adversaries unauthorized access to sensitive data or even allow them to execute system-level operations or code.\\n\\n## Remediations\\n\\n✅ Use Parameterized Queries\\n\\nAlways opt for parameterized queries over dynamically generated SQL queries to prevent SQL injection.\\n\\n\`\`\`go\\nrows, err := db.Query(\\"SELECT * FROM users WHERE userName = ?\\", userName)\\nif err != nil {\\n    return nil, err\\n}\\ndefer rows.Close()\\nfor rows.Next() {\\n  // ... process rows\\n}\\n\`\`\`\\n\\n✅ Avoid Direct User Input in Dynamic Queries\\n\\nIf there's an absolute need to formulate dynamic queries, ensure that direct user input is never utilized. Instead, leverage a map or dictionary containing valid values and determine them through a user-provided key.\\n\\nFor instance, certain database drivers do not support parameterized queries for operators like \`>\` or \`<\`. Instead of directly using user-input values, allow users to provide substitutes like \`gt\` for \`>\` and \`lt\` for \`<\`. Subsequently, use these alphabetical inputs to retrieve the actual operators for your query. Implement a similar approach for queries requiring non-parameterizable column or table names.\\n\\n## Resources\\n\\n- [OWASP SQL Injection Prevention Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html)\\n",
+      "documentation_url": "https://docs.bearer.com/reference/rules/go_gosec_sql_concat_sqli",
+      "line_number": 142,
+      "full_filename": "/tmp/bearer-scan/main.go",
+      "filename": ".",
+      "source": {
+        "start": 142,
+        "end": 142,
+        "column": {
+          "start": 15,
+          "end": 38
+        }
+      },
+      "sink": {
+        "start": 142,
+        "end": 142,
+        "column": {
+          "start": 15,
+          "end": 38
+        },
+        "content": "DB.Query(getProfileSql)"
+      },
+      "parent_line_number": 142,
+      "snippet": "DB.Query(getProfileSql)",
+      "fingerprint": "ca1a3415f7df18be8dec85c9d74eb890_5",
+      "old_fingerprint": "3350ddd230bfd667e45a89b42b9b1b39_5",
+      "code_extract": "\\trows, err := DB.Query(getProfileSql)"
     }
   ]
 }"

diff --git a/tests/go/gosec/sql/concat_sqli/testdata/main.go b/tests/go/gosec/sql/concat_sqli/testdata/main.go
@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"database/sql"
+	"fmt"
 	"os"
 )
 
@@ -127,3 +128,16 @@ func foo9() {
 	}
 	defer rows.Close()
 }
+
+var DB *sql.DB
+var err error
+
+func foo10(uid string) {
+	DB, err = database.Connect()
+
+	getProfileSql := fmt.Sprintf(`SELECT p.user_id, p.full_name, p.city, p.phone_number
+								FROM Profile as p,Users as u
+								where p.user_id = u.id
+								and u.id=%s`, uid) //here is the vulnerable query
+	rows, err := DB.Query(getProfileSql)
+}