Raw API concerns

[steebchen]

I have concerns about the current Prisma Client JS raw API.

The tagged template literal raw API is indeed nice to use:

var id = "title"
await client.raw`SELECT * FROM Post WHERE title = ${id}`

That works fine. No sql injections here because we handle that for the user.

However, since we’re encouraging users to use that syntax, I can imagine sooner or later a user will invoke the method directly instead and not change the variables in the template string to parameters for the function:

var id = "'title'; DROP TABLE User;"
await client.raw(`SELECT * FROM Post WHERE title = ${id}`)

and boom, SQL injection!

It seems that the editor's autocomplete feature also uses parentheses per default.

We need to think how we can prevent this from happening. My suggestion is that we only accept the template literal method for one method, and create a new function which makes it clear that the input does not get escaped, like rawNoEscape(...), similar to react's dangerouslySetInnerHtml.

[Sytten]

This should also be safe.

const result: number = await prisma.executeRaw('UPDATE User SET active = $1 WHERE email = $2;', [true, 'evil@gmail.com; DROP ALL TABLES'])

Looking at the code https://github.com/prisma/prisma/blob/master/src/packages/client/src/runtime/getPrismaClient.ts#L361 I don't see how example B can work / which code path it uses.

[thebiglabasky]

I did a few more tests and it turns out the the examples I had provided did work with proper escaping indeed.
I'll dig deeper into how this is all handled and post an update, but the general proposal for this specific issue is to mostly update our docs, for what I've opened a separate issue here.

[thebiglabasky]

After a few more tests and checking with the team, there are two main behaviors:

• queryRaw(sql) and executeRaw(sql) will only be executing a single statement, which means some injections won't work (the one with multiple queries trying to drop tables for example) but some will (unveiling additional data using UNION for example)
• queryRaw(sql, params) and executeRaw(sql, params) are translated to prepared statements which are inherently secure
• queryRaw`sql` and executeRaw`sql` are translated to prepared statements the same way and are also inherently secure

We need to keep the ability to execute SQL that is prepared elsewhere, leaving users the responsibility to sanitize those, as there are plenty of cases where custom logic can be used to generate an SQL query, or take them from another place without getting the parameters map details which would allow for preparing a statement.

Renaming the method being a breaking change, we will only update the docs to make it clear.

I'll thus close this issue.

[Sytten]

What needs to be added in the doc is the usage of the now exposed sql, join, etc that dev can use to create sanitized queries outside the templated method.