-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
SQL injection in Gorm With using first and find. #2517
Comments
|
@wahyuhadi @emirb please review #2561 |
…t SQL injection." This reverts commit e3cc5ea.
Can you please explain the revert that you did? There is no fix for this vulnerability issue? |
…o prevent SQL injection." (go-gorm#2674)" This reverts commit 81c17a7.
…ed parenthesis Revert "Revert "Fix go-gorm#2517 : Check for incomplete parentheses to prevent SQL injection." (go-gorm#2674)"
…j… (go-gorm#2519) Fix go-gorm#2517 : Check for incomplete parentheses to prevent SQL injection.
…t SQL injection." (go-gorm#2674) This reverts commit e3cc5ea.
Should this be reopened? I see there's an open pull request for resolving this (#2878). I ask because I am getting alerts that this package has security vulnerabilities. |
HI @ags799 |
@wahyuhadi does this affect where , find , raw , exec or is it limited to that use case ? I just want to know is this package escaping strings manually . Does it not use prepare statements / interpolating queries . |
i see the problem now . |
@baderkha I've tested where is ok, when using ("arg = ?",arg), gorm will use prepared statement, and the sql injection will not work. |
If using ("arg = " + arg), there will a risk for sql injection. |
This isn't actually a vulnerability. GORM lets you pass the WHERE as a string, in which case it places the string verbatim into the query. You might say this is bad API design as it invites the user to create SQL injection vulnerabilities if they don't sanitize the input. Keep in mind if you use the built-in parameterization (e.g. use "id = ?" and pass the ID as a parameter), there's no vulnerability. There's also no vulnerability if you pass the model struct as a parameter instead of a string. In short, this isn't a vulnerability in GORM. Here are some examples: SQL injection as in this issue - but the problem is in the calling code, not GORMThe only valid case to pass a string like this to GORM is when the entire WHERE condition string comes from your code, e.g. you're writing a complex WHERE condition that takes no user input.
Use parameters - no SQL injection as proper escaping is done by
|
Not agree @orishoshan!! GORM must bring default safe to the user, and if the user needs to control the WHERE clause it should have a method like React with dangerouslysetinnerhtml! |
Absolutely, this is bad API design, but it's not a vulnerability. |
i beleive it just needs to be communicated in the documentation to avoid doing something like that. I don't see this as a vulnerability nor bad api design per say. There should be documentation on how to avoid sql injection / bad practices... |
How is this not a vulnerability? I see a SQLi there. |
@jinzhu this is awesome . Thank you for your effort on providing documentation ! |
Thanks @jinzhu |
https://twitter.com/empijei/status/1295843334855032835 explains how the API could be made safer by requiring a constantString in places where it can potentially lead to a SQLi. |
…t SQL injection." (go-gorm#2674) This reverts commit c59ad19.
What version of Go are you using (
go version
)?latest
Which database and its version are you using?
postgress latest and gorm latest
Blind Sql injection localhost:8000/user?id=id=1)) or 1=1--
Need to runnable with GORM's docker compose config or please provides your config.
The text was updated successfully, but these errors were encountered: