Skip to content

Potential Issue Found: SQL Injection #49

Description

@bgeesaman

I've been doing source code analysis of certain types of public repos for a specific classes of problems, and I found a something in your repo from my research that you may want to take a look at.

Specifically:

result := DB.First(&user, ID)

When using GORM's db.First() method, if the second argument is a string that comes from user input instead of an int, it can provide a SQL Injection opportunity. GORM doesn't escape or automatically parameterize the query in this specific case. See https://gorm.io/docs/security.html#Inline-Condition for more details. The fix is to ensure that the second argument is always an integer or a struct.

Example:

id := c.Param("id")
db.First(&user, id) // If `id` is a string from attacker/user input, GORM performs direct concatenation

Fixed:

id := c.Param("id")
if parsedId, err := strconv.Atoi(id); err == nil {
    db.First(&user, parsedId) // Now `id` is guaranteed to be an integer and GORM handles it safely
} else {
    ... handle the error
}

Note: This research has taken some time to complete, so the commit I'm referencing is a few weeks old. You may have already fixed this issue in a later commit. If so, feel free to ignore/close. Just wanted to give you a heads up as a courtesy in case you found it helpful.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions