Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Revert "Fix code scanning alert no. 10: Database query built from user-controlled sources" #588

Merged
merged 1 commit into from
Jan 7, 2025
Merged

Revert "Fix code scanning alert no. 10: Database query built from user-controlled sources" #588

merged 1 commit into from
Jan 7, 2025

Conversation

adubovikov
Copy link
Member

Reverts #587

@adubovikov adubovikov merged commit 5d9073d into master Jan 7, 2025
2 checks passed
@adubovikov adubovikov deleted the revert-587-alert-autofix-10 branch January 7, 2025 11:46
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 7, 2025
View reviewed changes
data/service/userSettings.go
@@ -177,7 +177,7 @@

if err := ss.Session.Debug().
Table("user_settings").
Where("guid = ? AND username = ?", userObject.GUID, UserName).Find(&data).Error; err != nil {
Where(sqlWhere).Find(&data).Error; err != nil {

Check failure

Code scanning / CodeQL

Database query built from user-controlled sources High

This query depends on a
user-provided value
Loading
.
This query depends on a
user-provided value
Loading
.
This query depends on a
user-provided value
Loading
.

Copilot Autofix AI 20 days ago

To fix the problem, we should ensure that user-provided values are safely embedded into the SQL query using parameterized queries. GORM already provides a way to safely handle user input by using maps in the Where method. However, we should explicitly use parameterized queries to ensure that the values are properly escaped.

We will modify the Get, Delete, and Update methods in the UserSettingsService to use parameterized queries with named placeholders. This will ensure that the user-provided values are safely embedded into the SQL query.

Suggested changeset 1
data/service/userSettings.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/data/service/userSettings.go b/data/service/userSettings.go
--- a/data/service/userSettings.go
+++ b/data/service/userSettings.go
@@ -169,8 +169,11 @@
 
-	var sqlWhere = make(map[string]interface{})
+	var sqlWhere string
+	var args []interface{}
 
 	if !isAdmin {
-		sqlWhere = map[string]interface{}{"guid": userObject.GUID, "username": UserName}
+		sqlWhere = "guid = ? AND username = ?"
+		args = append(args, userObject.GUID, UserName)
 	} else {
-		sqlWhere = map[string]interface{}{"guid": userObject.GUID}
+		sqlWhere = "guid = ?"
+		args = append(args, userObject.GUID)
 	}
@@ -179,3 +182,3 @@
 		Table("user_settings").
-		Where(sqlWhere).Find(&data).Error; err != nil {
+		Where(sqlWhere, args...).Find(&data).Error; err != nil {
 		return data, err
@@ -189,8 +192,11 @@
 
-	var sqlWhere = make(map[string]interface{})
+	var sqlWhere string
+	var args []interface{}
 
 	if !isAdmin {
-		sqlWhere = map[string]interface{}{"guid": userObject.GUID, "username": UserName}
+		sqlWhere = "guid = ? AND username = ?"
+		args = append(args, userObject.GUID, UserName)
 	} else {
-		sqlWhere = map[string]interface{}{"guid": userObject.GUID}
+		sqlWhere = "guid = ?"
+		args = append(args, userObject.GUID)
 	}
@@ -199,4 +205,3 @@
 		Table("user_settings").
-		Where(sqlWhere).
-		Delete(model.TableUserSettings{}).Error; err != nil {
+		Where(sqlWhere, args...).Delete(model.TableUserSettings{}).Error; err != nil {
 		return err
@@ -210,8 +215,11 @@
 
-	var sqlWhere = make(map[string]interface{})
+	var sqlWhere string
+	var args []interface{}
 
 	if !isAdmin {
-		sqlWhere = map[string]interface{}{"guid": userObject.GUID, "username": UserName}
+		sqlWhere = "guid = ? AND username = ?"
+		args = append(args, userObject.GUID, UserName)
 	} else {
-		sqlWhere = map[string]interface{}{"guid": userObject.GUID}
+		sqlWhere = "guid = ?"
+		args = append(args, userObject.GUID)
 	}
@@ -222,3 +230,3 @@
 		Model(&model.TableUserSettings{}).
-		Where(sqlWhere).Update(userObject).Error; err != nil {
+		Where(sqlWhere, args...).Update(userObject).Error; err != nil {
 		return err
EOF
@@ -169,8 +169,11 @@

var sqlWhere = make(map[string]interface{})
var sqlWhere string
var args []interface{}

if !isAdmin {
sqlWhere = map[string]interface{}{"guid": userObject.GUID, "username": UserName}
sqlWhere = "guid = ? AND username = ?"
args = append(args, userObject.GUID, UserName)
} else {
sqlWhere = map[string]interface{}{"guid": userObject.GUID}
sqlWhere = "guid = ?"
args = append(args, userObject.GUID)
}
@@ -179,3 +182,3 @@
Table("user_settings").
Where(sqlWhere).Find(&data).Error; err != nil {
Where(sqlWhere, args...).Find(&data).Error; err != nil {
return data, err
@@ -189,8 +192,11 @@

var sqlWhere = make(map[string]interface{})
var sqlWhere string
var args []interface{}

if !isAdmin {
sqlWhere = map[string]interface{}{"guid": userObject.GUID, "username": UserName}
sqlWhere = "guid = ? AND username = ?"
args = append(args, userObject.GUID, UserName)
} else {
sqlWhere = map[string]interface{}{"guid": userObject.GUID}
sqlWhere = "guid = ?"
args = append(args, userObject.GUID)
}
@@ -199,4 +205,3 @@
Table("user_settings").
Where(sqlWhere).
Delete(model.TableUserSettings{}).Error; err != nil {
Where(sqlWhere, args...).Delete(model.TableUserSettings{}).Error; err != nil {
return err
@@ -210,8 +215,11 @@

var sqlWhere = make(map[string]interface{})
var sqlWhere string
var args []interface{}

if !isAdmin {
sqlWhere = map[string]interface{}{"guid": userObject.GUID, "username": UserName}
sqlWhere = "guid = ? AND username = ?"
args = append(args, userObject.GUID, UserName)
} else {
sqlWhere = map[string]interface{}{"guid": userObject.GUID}
sqlWhere = "guid = ?"
args = append(args, userObject.GUID)
}
@@ -222,3 +230,3 @@
Model(&model.TableUserSettings{}).
Where(sqlWhere).Update(userObject).Error; err != nil {
Where(sqlWhere, args...).Update(userObject).Error; err != nil {
return err
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
data/service/userSettings.go Dismissed Show dismissed Hide dismissed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@adubovikov