search

tokio-rs / rdbc

Rust DataBase Connectivity (RDBC) :: Common Rust API for database drivers
Apache License 2.0
566 stars 25 forks source link

Protect against SQL injection attacks #17

Open andygrove opened 4 years ago

rumatoest commented 4 years ago

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

mamcx commented 4 years ago

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)

rumatoest commented 4 years ago

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 4 years ago

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)

rumatoest commented 4 years ago

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 4 years ago

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.

rumatoest commented 4 years ago

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 4 years ago

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