tokio-rs/rdbc

Protect against SQL injection attacks

andygrove opened this issue · 8 comments

Protect against SQL injection attacks

Probably should have trait that could convert any DB compatible type to an escaped string compatible with particular DB.

mamcx commented

No, the best is to call the parametrized API of the driver. The trouble is that different DBs support different marks for what is parameter. Also, some have hard limits in how many can be constructed, that trip you for example if wanna do a safe mass insert.

Is good idea the db layer pick a style of marks and convert it (and also: Allow names, not only positions)

No, the best is to call the parametrized API of the driver.

Why you could not have this API wrapped into appropriate trait?

mamcx commented

Because the driver already do that, and it could do much more that just escaping (ie: You can send a json as string from the app layer and the db take it as json. If you escape the json, you corrupt it)

You can send a json as string from the app layer and the db take it as json

This is bad example. If you are sending JSON as string it has to be another Rust type that implements Deref<String>. In other cases string must be escaped. If you do not want to escape anything, you have to use string concatenation on rust side not via result set.

mamcx commented

In other cases string must be escaped. If you do not want to escape anything, you have to use string concatenation on rust side not via result set.

But is still a use case. And also is duplicating what the driver do. Using params is what is expected by any developer using a db lib. In fact i have some code that do exactly what you say, and get in wrong input anyway when the DB schema was changed OUTSIDE the app code. You can't assume what the types in rust are are ALWAYS what the db have.

P.D: Having an escape utility is good idea, but is tangential to this.

Escape utility should be embedded into prepare statement generation. But there also shold be separate trait in order if someone will try to implement some kind of dynamic query builder.

95th commented

I agree with @mamcx here. This should be done by parameterized statements.