How to use glue_data_sql to write safe parameterized queries on an SQL server database?

Question

The problem

I want to write a wrapper around some DBI functions that allows safe execution of parameterized queries. I've found the this resource that explains how to use the glue package to insert parameters into an SQL query. However, there seem to be two distinct ways to use the glue package to insert parameters:

1. Method 1 involves using ? in the sql query where the parameters need to be inserted, and then subsequently using dbBind to fill them in. Example from the link above:

library(glue)
library(DBI)

airport_sql <- glue_sql("SELECT * FROM airports WHERE faa = ?")
airport <- dbSendQuery(con, airport_sql)

dbBind(airport, list("GPT"))
dbFetch(airport)

2. Method 2 involves using glue_sql or glue_data_sql to fill in the parameters by itself (no use of dbBind). Again an example from the link above:

airport_sql <- 
  glue_sql(
    "SELECT * FROM airports WHERE faa IN ({airports*})", 
    airports = c("GPT", "MSY"),
    .con = con
  )

airport <- dbSendQuery(con, airport_sql)

dbFetch(airport)

I would prefer using the second method because this has a lot of extra functionality such as collapsing multiple values for an in statement in the where clause of an sql statement. See second example above for how that works (note the * after the parameter which indicates it must be collapsed). The question is: is this safe against SQL injection? (Are there other things I need to worry about?)

My code

This is currently the code I have for my wrapper.

paramQueryWrapper <- function(
  sql,
  params = NULL,
  dsn    = standard_dsn,
  login  = user_login,
  pw     = user_pw
){

  if(missing(sql) || length(sql) != 1 || !is.character(sql)){
    stop("Please provide sql as a character vector of length 1.")
  }

  if(!is.null(params)){
    if(!is.list(params))       stop("params must be a (named) list (or NULL).")
    if(length(params) < 1)     stop("params must be either NULL, or contain at least one element.")
    if(is.null(names(params)) || any(names(params) == "")) stop("All elements in params must be named.")
  }

  con <- DBI::dbConnect(
    odbc::odbc(),
    dsn      = dsn,
    UID      = login,
    PWD      = pw
  )
  on.exit(DBI::dbDisconnect(con), add = TRUE)


  # Replace params with corresponding values and execute query

  sql <- glue::glue_data_sql(.x = params, sql, .con = con)

  query <- DBI::dbSendQuery(conn = con, sql)
  on.exit(DBI::dbClearResult(query), add = TRUE, after = FALSE)

  return(tibble::as_tibble(DBI::dbFetch(query)))

}

My question

Is this safe against SQL injection? Especially since I am not using dbBind.

Epilogue

I know that there already exists a wrapper called dbGetQuery that allows parameters (see this question for more info - look for the answer by @krlmlr for an example with a parameterized query). But this again relies on the first method using ? which is much more basic in terms of functionality.

Meemeece

I see SQL injection mentioned in the DBI documentation: cran.r-project.org/web/packages/DBI/DBI.pdf – Quinlan

glue_sql documentation also mentions injection (on top of dbBind): glue.tidyverse.org/reference/glue_sql.html – Quinlan

@Quinlan 1. About the DBI documentation. This mentions SQL injection in several functions amongst which dbBind. But as I have mentioned in my answer this function does not allow named parameters for an SQL server database. So unless someone can explain to me how to use dbBind with named parameters on sql server this is irrelevant. 2. About the documentation on glue_sql. This literally only says parameterized queries are safest, implying that use of dbBind is better. But it doesn't mention anything about the safety of using glue_sql by itself. Again not answering my question. – Meemeece

Looking at this tweet, it seems that glue_sql in principle is not safe against SQL injection – Gunfight