debug_query and SQL Injection. Why does this not inject?

[B4rc1]

Hi, I am trying to implement a service where the frontend can filter based on a number of criterias, the library i use for the front end allows me to export the filter as as SQL WHERE clause.
The simplest approach would be for the backend to accept a string that then gets appended to the WHERE clause of a select. This comes with the danger of an SQL injection.
My Question is if diesel protects against this sort of attack. And how can I debug this?
My code is the following:

pub fn find_song_filter_by(conn: &mut SqliteConnection, filter: String) -> Vec<models::Song> {

    let query = models::SongHasTag::table().inner_join(schema::tag::table).inner_join(schema::song::table).filter(sql::<sql_types::Bool>(&filter)).select(models::Song::as_select());
    info!("{:?}", debug_query::<diesel::sqlite::Sqlite, _>(&query));
    return query.load(conn).expect("");
}

If i try the following input: find_song_filter_by(conn, r#"tag_name= "tag1"; INSERT INTO tag VALUES ("we are in")"#.into())
This gets logged:

Query { sql: "SELECT `song`.`id` FROM ((`song_hg`.`tag_name` = `tag`.`name`)) INNER JOIN `song` ON (`song_has_tag`.`song_id` = `song`.`id`)) WHERE tag_name= \"tag1\"; INSERT INTO tag VALUES (\"we are in\")", binds: [] }

Which (if I understand correctly) corresponds to this SQL query:

SELECT `song`.`id` FROM ((`song_has_tag` INNER JOIN `tag` ON (`song_has_tag`.`tag_name` = `tag`.`name`)) INNER JOIN `song` ON (`song_has_tag`.`song_id` = `song`.`id`)) WHERE tag_name= "tag1"; INSERT INTO tag VALUES ("we are in")

If I execute this query manually against my SQL database, the tag "we are in" gets created, demonstrating an SQL injection.

However, executing the returned query with diesel, no such tag is created, which leads me to believe that diesel uses extra type information (the fact that i specified filter(sql::<sql_types::Bool>(&filter))) to execute the query.

How does the executed query actually look like if debug_query cannot be trusted? Is what I'm doing supported/safe with diesel?

[weiznich]

Diesel uses prepared statements for any query. That usually prevents any form of SQL injections as parameters are send separate from the query.

In this psecific case it does not help as you literally pass in a user supplied string as query part via dsl::sql. That function and diesel::sql_query are exceptions in so far that they do not process the strings provided by the user in any way, so if you pass in a string that is constructed via format! your will likely run into SQL injection problems. It does not seem to work for your particular example as you mixed up the quotation marks. SQL uses ' to quote strings and because diesel always will only execute the first query in a particular statement (at least unless you call the SimpleConnection::batch_execute function).

To summarize that: I highly recommend not to accept raw strings as query additions in that way, it will result in injection like problems. Instead checkout the "Composing Applications", it demonstrates different ways to construct composable queries with diesel.

[B4rc1]

Okay so as far as I understand, the injection could cause DDOS attacks (e.g. by using an expensive regex as a filter) but cannot be used to modify data since diesel only executes the first query and the user supplied string gets appended to the WHERE part of a SELECT statement and there is (as far as i know) no way to change a select statement into a modifying statement if you only control everything after the WHERE part without starting a new query. Does this sound correct?

So for a MVP version of my application this should be fine but a more limited/abstracted way should be implemented before release.

Thank you for the clarification and feedback :)

[weiznich]

Just to make this clear: We do not guarantee that only the first statement is executed, it's merely the current behavior in this case. You shouldn't rely on that as security guarantee.