0

I am working on a PHP application based on MS SQL that, among other things, can add a column to a table, either bit, int or varchar. My current code is similar to this:

$sql = "ALTER TABLE myTable ADD ? ".$type;
$values = array($columnname);
if($type == "varchar")
    $sql = $sql."(".$length.")";
$sql = $sql.";";
sqlsrv_query($conn, $sql, $values);

However, I only get back an error that tells me there's an incorrect syntax near '@P1'. I thought about inserting the column name into the string directly, but I'm afraid of SQL injection as it is a text input. I'm not as worried about it for the type and the length as those are a select and a number input.

I am hoping one of you can give me some advice on how to add the column without exposing myself to SQL injection. Thank you in advance!

(As this question has been marked a 'possible duplicate' I'd like to clarify that I'm specifically asking about adding a new column without exposing myself to SQL injection, not just inserting values into a table. I hope I made this clear.)

pttg
  • 111
  • 4
  • Possible duplicate of [How can I prevent SQL injection in PHP?](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Sanu0786 Nov 23 '18 at 07:17
  • You probably cannot use parameters to specify identifier names (table names, column names, etc). – Salman A Nov 23 '18 at 07:23
  • @SalmanA Yes, I suspected that as well, but I'm not sure how else to do it without exposing myself to SQL injection. – pttg Nov 23 '18 at 07:25

1 Answers1

1

As comments above have stated, you can use a parameter placeholder only to substitute for a scalar value — that is, where you would otherwise use a single quoted string, quoted date/time, or numeric literal.

Parameters don't work for identifiers like table names or column names, nor SQL keywords, nor SQL expressions, nor lists of values (you would have to use one placeholder for each scalar value in a list).

For cases where you want to make a column name or SQL keywords dynamic as you are trying to do, you will be vulnerable to SQL injection unless you make sure to restrict the dynamic part to unharmful strings.

For example, you can do this with delimiting and escaping.

$columnname = str_replace("]", "]]", $columnname);
$sql = "ALTER TABLE myTable ADD [$columnname] $type";

MS SQL Server uses square brackets to delimit an identifier that may contain special characters. The str_replace() is just in case the column name itself contains square brackets. Read https://sqlsunday.com/2014/09/21/identifiers-in-tsql/ for more explanation.

What about $type? How can we make sure that's safe? The other technique is whitelisting. To make $type safe, you would need to check it against a list of allowed values.

switch ($type) {
case "int":
  // OK, nothing to change
  break;
case "varchar":
  $type = "varchar($length)"
  break;
// ...add cases for other allowed types...
default:
  die("Unrecognized type: $type");
}

You can either die with an error in the default clause, or you can set $type to a reasonable default. Depends on how you want to handle it.

You might ask, Why so much code? Why can't this be simpler?

Because you require the software to do something that is dynamic. You're trying to make code do the right thing no matter what some user inputs. If you didn't need to support user input, you could make the column name and type fixed, and then it would be simple.

Bill Karwin
  • 538,548
  • 86
  • 673
  • 828
  • 1
    Well stated. And I would add to this for OP - perhaps the issue is the design of a system that allows (encourages?) a user to alter the schema. That ability alone requires permissions that users are not typically given. – SMor Nov 25 '18 at 13:19