Lack of tests & docs regarding SQL injection vulnerabilities

[ewindisch]

The reason that parametrized queries exist are to avoid the SQL injections that are inevitable from parsing strings. This project appears to simply attempt to circumvent that security mechanism (even if it does not, ultimately). How this deals with SQL injections should be documented. Tests should cover SQL injections as well.

[felixfbecker]

Could you please explain in detail how this project circumvents that mechanism?

[ewindisch]

I should underscore appears. Reading the statements would appear to enable SQL injections. By not having explicit parametrized queries and relying on string-parsing, we fail to adhere to best practices that prevent SQL injection. The implementation seems to likely account for this -- but explaining to users what it does and how it protects against SQL injections would be useful for observers and auditors.

Unit tests should definitely cover SQL injection attacks as this is (at least to me) the major purpose of having parametrized queries in SQL (secondarily, for having prepared/reusable statements).

[felixfbecker]

This project does not do any text parsing, it just takes the strings and values and packs them in an object as described here: https://github.com/felixfbecker/node-sql-template-strings#how-it-works. Any specific improvements you would do to the documentation?

[felixfbecker]

Closing for lack of response