Fix #2517 : Check for incomplete parentheses to prevent SQL injection.

[herpiko]

What did this pull request do?

Fix for #2517

[codecov-io]

Codecov Report

Merging #2519 into master will increase coverage by 0.08%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2519      +/-   ##
==========================================
+ Coverage   79.34%   79.42%   +0.08%     
==========================================
  Files          24       24              
  Lines        3452     3466      +14     
==========================================
+ Hits         2739     2753      +14     
  Misses        612      612              
  Partials      101      101

Impacted Files	Coverage Δ
scope.go	87.22% <100%> (+0.21%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6b790f...8215224. Read the comment docs.

[herpiko]

@emirb I've applied your comment. Can you check it again please?

[emirb]

Thanks @herpiko.

[wahyuhadi]

hi All
this issue still exists

[sneko]

@wahyuhadi you say the issue still exist. Could you please provide more information or an example of the code that is vulnerable please?

Thank you,

[wahyuhadi]

@sneko
You can see in here
https://github.com/wahyuhadi/gorm-sqlInjection

[maeglindeveloper]

@wahyuhadi ,

I see that you are passing, as a primary key, an interface from gin.Context.
When querying with First using inline conditions, it is said that it only works with integer primary key 👍

Since there is no prepare statement in this usecase, it seems logical that there is an issue in this example, wdyt ?
Maybe I'm wrong.

// Get by primary key (only works for integer primary key)
db.First(&user, 23)
//// SELECT * FROM users WHERE id = 23;
// Get by primary key if it were a non-integer type
db.First(&user, "id = ?", "string_primary_key")
//// SELECT * FROM users WHERE id = 'string_primary_key';

// Plain SQL
db.Find(&user, "name = ?", "jinzhu")
//// SELECT * FROM users WHERE name = "jinzhu";

db.Find(&users, "name <> ? AND age > ?", "jinzhu", 20)
//// SELECT * FROM users WHERE name <> "jinzhu" AND age > 20;

// Struct
db.Find(&users, User{Age: 20})
//// SELECT * FROM users WHERE age = 20;

// Map
db.Find(&users, map[string]interface{}{"age": 20})
//// SELECT * FROM users WHERE age = 20;

But still, it seems necessary to use the specific gorm pattern to avoid SQL injection.
I do have a doubt as well regarding the strenght of the library either regarding that topic.

@jinzhu maybe you can lightened us a bit more ? :)

[wahyuhadi]

@maeglindeveloper

The injection id=id=1)) or 1=1-- , i call this double param injection

If you change the injection id=email=some@email.com)) or 1=1--

The query will change with where email=some@email.con

[maeglindeveloper]

@wahyuhadi yes, I understood first time ;)