feat: improve SQLi (#255)

Bearer · Feb 12, 2024 · 7a20965 · 7a20965
1 parent 675aca2
commit 7a20965
Show file tree

Hide file tree

Showing 33 changed files with 976 additions and 7 deletions.
diff --git a/.envrc.example b/.envrc.example
@@ -1,3 +1,5 @@
 export BEARER_VERSION=latest
 export BEARER_WORKSPACE=$PWD/../bearer
 export GO_EXEC=go
+export BEARER_DISABLE_VERSION_CHECK=true
+export BEARER_DISABLE_DEFAULT_RULES=true
diff --git a/rules/go/gosec/sql/concat_sqli.yml b/rules/go/gosec/sql/concat_sqli.yml
@@ -1,9 +1,16 @@
 imports:
   - go_shared_lang_dynamic_request_input
+  - go_shared_lang_dynamic_input
 patterns:
   - pattern: |
-      $<DB>.Query($<INPUT>)
+      $<DB>.$<METHOD>($<INPUT>)
     filters:
+      - variable: METHOD
+        values:
+          - Query
+          - QueryRow
+          - Prepare
+          - Exec
       - variable: INPUT
         detection: go_gosec_sql_concat_sqli_unsanitized_input
       - not:
@@ -19,18 +26,30 @@ patterns:
           - variable: DB
             detection: go_gosec_sql_concat_sqli_sql_db_begin
             scope: cursor
+          - variable: DB
+            regex: (?i).*(db|database)
   - pattern: |
-      $<DB>.QueryContext($<...>$<INPUT>)
+      $<DB>.$<METHOD>($<_>, $<INPUT>)
     filters:
+      - variable: METHOD
+        values:
+          - QueryContext
+          - PrepareContext
+          - ExecContext
       - variable: 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_open
             scope: cursor
           - variable: DB
             detection: go_gosec_sql_concat_sqli_sql_db_begin
             scope: cursor
+          - variable: DB
+            string_regex: (?i).*(db|database)
 auxiliary:
   - id: go_gosec_sql_concat_sqli_input_sprintf_sanitizer
     patterns:
@@ -46,6 +65,9 @@ auxiliary:
     patterns:
       - pattern: $<INPUT>
         filters:
+          - variable: INPUT
+            detection: go_shared_lang_dynamic_input
+            scope: cursor
           - variable: INPUT
             detection: go_shared_lang_dynamic_request_input
             scope: cursor

diff --git a/rules/go/shared/lang/dynamic_input.yml b/rules/go/shared/lang/dynamic_input.yml
@@ -26,6 +26,7 @@ auxiliary:
       - os.Args
       - os.Getenv()
       - func $<_>($<...>$<!>$<ARG> $<_>$<...>)
+      - func ($<...>) $<_>($<...>$<!>$<_> $<_>$<...>)$<...>
 metadata:
   description: "Go dynamic input."
   id: go_shared_lang_dynamic_input
diff --git a/tests/go/gosec/sql/concat_sqli/test.js b/tests/go/gosec/sql/concat_sqli/test.js
@@ -1,4 +1,8 @@
-const { createInvoker, getEnvironment } = require("../../../../helper.js")
+const {
+  createInvoker,
+  createNewInvoker,
+  getEnvironment,
+} = require("../../../../helper.js")
 const { ruleId, ruleFile, testBase } = getEnvironment(__dirname)
 
 describe(ruleId, () => {
@@ -14,3 +18,236 @@ describe(ruleId, () => {
     expect(invoke(testCase)).toMatchSnapshot()
   })
 })
+
+describe(`${ruleId}-xx`, () => {
+  const invoke = createNewInvoker(ruleId, ruleFile, testBase)
+
+  test("test", () => {
+    const testCase = "function.go"
+
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test1", () => {
+    const testCase = "test01.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test2", () => {
+    const testCase = "test02.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test3", () => {
+    const testCase = "test03.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test4", () => {
+    const testCase = "test04.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test5", () => {
+    const testCase = "test05.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test6", () => {
+    const testCase = "test06.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test7", () => {
+    const testCase = "test07.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test8", () => {
+    const testCase = "test08.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test9", () => {
+    const testCase = "test09.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test10", () => {
+    const testCase = "test10.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test11", () => {
+    const testCase = "test11.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test12", () => {
+    const testCase = "test12.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test13", () => {
+    const testCase = "test13.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test14", () => {
+    const testCase = "test14.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test15", () => {
+    const testCase = "test15.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+})
+
+describe(`${ruleId}-1xx`, () => {
+  const invoke = createNewInvoker(ruleId, ruleFile, testBase)
+
+  test("test101", () => {
+    const testCase = "test101.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test102", () => {
+    const testCase = "test102.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test103", () => {
+    const testCase = "test103.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test104", () => {
+    const testCase = "test104.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test105", () => {
+    const testCase = "test105.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test106", () => {
+    const testCase = "test106.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test107", () => {
+    const testCase = "test107.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test108", () => {
+    const testCase = "test108.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test109", () => {
+    const testCase = "test109.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test110", () => {
+    const testCase = "test110.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test111", () => {
+    const testCase = "test111.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+
+  test("test112", () => {
+    const testCase = "test112.go"
+    const results = invoke(testCase)
+
+    expect(results.Missing).toEqual([])
+    expect(results.Extra).toEqual([])
+  })
+})
diff --git a/tests/go/gosec/sql/concat_sqli/testdata/function.go b/tests/go/gosec/sql/concat_sqli/testdata/function.go
@@ -0,0 +1,73 @@
+package sqli
+
+import (
+	"database/sql"
+	"fmt"
+	"log"
+
+	"github.com/foo/utils/database"
+)
+
+var DB *sql.DB
+var err error
+
+type Profile struct {
+	Uid         int
+	Name        string
+	City        string
+	PhoneNumber string
+}
+
+func NewProfile() *Profile {
+	return &Profile{}
+}
+
+func (p *Profile) UnsafeQueryGetData(uid string) error {
+
+	/* this funciton use to get data Profile from database with vulnerable query */
+	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
+
+	// bearer:expected go_gosec_sql_concat_sqli
+	rows, err := DB.Query(getProfileSql)
+	if err != nil {
+		return err
+	}
+	defer rows.Close()
+
+	for rows.Next() {
+		err = rows.Scan(&p.Uid, &p.Name, &p.City, &p.PhoneNumber)
+		if err != nil {
+			log.Printf("Row scan error: %s", err.Error())
+			return err
+		}
+	}
+	return nil
+}
+
+func (p *Profile) SafeQueryGetData(uid string) error {
+	DB, err = database.Connect()
+
+	const (
+		getProfileSql = `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=?`
+	)
+
+	stmt, err := DB.Prepare(getProfileSql) //prepare statement
+	if err != nil {
+		return err
+	}
+
+	defer stmt.Close()
+	err = stmt.QueryRow(uid).Scan(&p.Uid, &p.Name, &p.City, &p.PhoneNumber)
+	if err != nil {
+		return err
+	}
+	return nil
+}