-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Detecting possible SQL Injection Vulernerabilties #71
Comments
I think phpstan-dba will have different views/solutions on this problem
|
While I'd prefer
|
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
On your Todos, you have "security rule: detect possible sql injections".
May I suggest using the
literal-string
type, as it's very simple, and is the only way of being sure an Injection Vulnerability cannot exist.It basically says the SQL must be written by the developer (i.e. it cannot use any dangerous user input).
All user input should use
parameters
, on the basis that it's easy for escaping to be forgotten, or go wrong:Notice how the developer forgot to quote the value, so the attacker could easily do
example.com/?id=id
(or something a bit more complex, like using a UNION)... and that's before we get into character set issues, and thesql_mode
optionNO_BACKSLASH_ESCAPES
.If a developer wants to use the
IN()
operator, that's fine, as they should still use parameters, maybe with something safe like:or maybe use a function like this, to make their code easier to read:
And if a developer is doing something like allowing the user to choose how the results are sorted, they should limit the user to only those fields they are allowed to sort by (and nothing more), e.g.
There may be cases where the field names cannot be defined in the source code (some complex CMS'es do this, for "reasons")... in which case, they can do something like
@phpstan-ignore-line
... this should be very rare, and will be flagged as something to check during an audit (a good thing).I must admit it's been a while since I've used PDO and MySQLi directly, but I think it's the
$sql
argument in these functions that will need checking:The text was updated successfully, but these errors were encountered: