Skip to content

Hack me. #1009

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Hack me. #1009

wants to merge 1 commit into from

Conversation

andream16
Copy link
Contributor

No description provided.

Copy link

@smithy-cloud-plexor smithy-cloud-plexor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smithy scan results

Comment on lines +1 to +192
}
query += id // No quotes or sanitization!
}
query += ")"

_, err := db.Exec(query)
return err
}

// VULNERABLE: Complex query with multiple injection points
func getFilteredUsers(minAge, maxAge, city, profession string) ([]User, error) {
var users []User

// This is vulnerable to SQL injection in multiple places!
query := "SELECT id, username, email FROM users WHERE 1=1"

if minAge != "" {
query += " AND age >= " + minAge
}
if maxAge != "" {
query += " AND age <= " + maxAge
}
if city != "" {
query += " AND city = '" + city + "'"
}
if profession != "" {
query += " AND profession = '" + profession + "'"
}

rows, err := db.Query(query)
if err != nil {
return nil, err
}
defer rows.Close()

for rows.Next() {
var user User
err := rows.Scan(&user.ID, &user.Username, &user.Email)
if err != nil {
return nil, err
}
users = append(users, user)
}
return users, nil
}

// VULNERABLE: Raw SQL execution from user input
func executeCustomQuery(customSQL string) ([]map[string]interface{}, error) {
// This is extremely dangerous - allows arbitrary SQL execution!
rows, err := db.Query(customSQL)
if err != nil {
return nil, err
}
defer rows.Close()

columns, err := rows.Columns()
if err != nil {
return nil, err
}

var results []map[string]interface{}
for rows.Next() {
values := make([]interface{}, len(columns))
valuePtrs := make([]interface{}, len(columns))
for i := range values {
valuePtrs[i] = &values[i]
}

if err := rows.Scan(valuePtrs...); err != nil {
return nil, err
}

result := make(map[string]interface{})
for i, col := range columns {
result[col] = values[i]
}
results = append(results, result)
}

return results, nil
}

type User struct {
ID int `json:"id"`
Username string `json:"username"`
Email string `json:"email"`
}

func main() {
var err error
db, err = sql.Open("mysql", "user:password@tcp(localhost:3306)/testdb")
if err != nil {
log.Fatal(err)
}
defer db.Close()

http.HandleFunc("/user", getUserHandler)

fmt.Println("Server starting on :8080")
log.Fatal(http.ListenAndServe(":8080", nil))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  Use of net/http serve function that has no support for setting timeouts
Original Description: Use of net/http serve function that has no support for setting timeouts Help: Use of net/http serve function that has no support for setting timeouts Severity: MEDIUM Confidence: HIGH
FindingLink
Instancedogfooding-smithy-code-fed5d9
Toolgosec

Comment on lines +1 to +31
package main

import (
"database/sql"
"fmt"
"log"
"net/http"

_ "github.com/go-sql-driver/mysql"
)

var db *sql.DB

// VULNERABLE: Direct string concatenation in SQL query
func getUserByID(userID string) (*User, error) {
// This is vulnerable to SQL injection!
query := "SELECT id, username, email FROM users WHERE id = " + userID

row := db.QueryRow(query)
var user User
err := row.Scan(&user.ID, &user.Username, &user.Email)
if err != nil {
return nil, err
}
return &user, nil
}

// VULNERABLE: String formatting in SQL query
func loginUser(username, password string) bool {
// This is vulnerable to SQL injection!
query := fmt.Sprintf("SELECT id FROM users WHERE username = '%s' AND password = '%s'", username, password)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  SQL string formatting
Original Description: SQL string formatting Help: SQL string formatting Severity: MEDIUM Confidence: HIGH
FindingLink
Instancedogfooding-smithy-code-fed5d9
Toolgosec

// This is vulnerable to SQL injection!
query := "SELECT id, username, email, role FROM users WHERE id = " + userID

row := db.QueryRow(query)

Check failure

Code scanning / CodeQL

Database query built from user-controlled sources High

This query depends on a
user-provided value
.

Copilot Autofix

AI 11 days ago

To fix the issue, the code should use prepared statements with placeholder parameters to safely embed user-provided values into SQL queries. Prepared statements ensure that user input is treated as data rather than executable code, mitigating SQL injection risks.

For the getUserHandler function:

  1. Replace the direct string concatenation with a prepared statement using ? placeholders for user-provided values.
  2. Use the db.QueryRow method with the prepared query and pass the user-provided userID as a parameter.

This change requires no additional imports or definitions, as the database/sql package already supports prepared statements.


Suggested changeset 1
hack/main.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/hack/main.go b/hack/main.go
--- a/hack/main.go
+++ b/hack/main.go
@@ -66,5 +66,5 @@
 	// This is vulnerable to SQL injection!
-	query := "SELECT id, username, email, role FROM users WHERE id = " + userID
+	query := "SELECT id, username, email, role FROM users WHERE id = ?"
 
-	row := db.QueryRow(query)
+	row := db.QueryRow(query, userID)
 	var id int
EOF
@@ -66,5 +66,5 @@
// This is vulnerable to SQL injection!
query := "SELECT id, username, email, role FROM users WHERE id = " + userID
query := "SELECT id, username, email, role FROM users WHERE id = ?"

row := db.QueryRow(query)
row := db.QueryRow(query, userID)
var id int
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant