A Microsot Bug in Dynamic Comlumns?

[valentin tihomirov]

Hello,

-=PREHISTORY=-

Occasionally, I wanted to rewrite my UPDATE code to prevent SQL injection. I
generated the SQL query

UPDATE tt
SET
@p1_name = @p1_val
, @p2_name = @p2_val
, ...

along with the pairs of paramz:

new SqlParameter(formalName, actualName);
new SqlParameter(formalValue, actualValue);

for every column. Now, user can send name-value pairs the corresponding
columns will be updated safely. However, this fails if a value is longer
than 4 chars:

"System.Data.SqlClient.SqlException: String or binary data would be
truncated. The statement has been terminated."

Note, the code still works if the column names are not parametrized. As this
is weired, so I have decided to trial in pure SQL.
-=SQL=-

Here is the code to execute:

DECLARE @val varchar(100)
SELECT @val ='vvvv' -- making val longer breaks the code
exec sp_executesql N'update tt SET @name=@val'
, N'@name varchar(4), @val varchar(100)'
, 'col1', @val
Making the value one char longer will end up in the suspiciously famous "Msg
8152, Level 16, State 14, Line 1 String or binary data would be truncated."

THE INTERESTING thing is that the maximal allowed @val length is determined
by the @name parameter definition! Try yourslf by changing the declaration
varchar(4) -varchar(5) and this will allow entering 5-symbol values.

Another bug observed is possibly related to the this one. The table data is
not affected even when no error is encountered. The value is taken only when
column name is not parametrized, that is, 'UPDATE tt SET col1 = @val' is
submitted.

Am I missing something?

[Alex Kuznetsov]

On Oct 19, 10:37 am, "valentin tihomirov" <[e-mail]:

Hello,

-=PREHISTORY=-

Occasionally, I wanted to rewrite my UPDATE code to prevent SQL injection. I
generated the SQL query

UPDATE tt
SET
@p1_name = @p1_val
, @p2_name = @p2_val
, ...

along with the pairs of paramz:

new SqlParameter(formalName, actualName);
new SqlParameter(formalValue, actualValue);

for every column. Now, user can send name-value pairs the corresponding
columns will be updated safely. However, this fails if a value is longer
than 4 chars:

"System.Data.SqlClient.SqlException: String or binary data would be
truncated. The statement has been terminated."

Note, the code still works if the column names are not parametrized. As this
is weired, so I have decided to trial in pure SQL.

-=SQL=-

Here is the code to execute:

DECLARE @val varchar(100)
SELECT @val ='vvvv' -- making val longer breaks the code
exec sp_executesql N'update tt SET @name=@val'
, N'@name varchar(4), @val varchar(100)'
, 'col1', @val

Making the value one char longer will end up in the suspiciously famous "Msg
8152, Level 16, State 14, Line 1 String or binary data would be truncated."

THE INTERESTING thing is that the maximal allowed @val length is determined
by the @name parameter definition! Try yourslf by changing the declaration
varchar(4) -varchar(5) and this will allow entering 5-symbol values.

Another bug observed is possibly related to the this one. The table data is
not affected even when no error is encountered. The value is taken only when
column name is not parametrized, that is, 'UPDATE tt SET col1 = @val' is
submitted.

that's not a bug, it works as designed. The right approach might be
along the following lines:

UPDATE tt
SET
column1 = COALESCE(@p1_val, column1),
column2 = COALESCE(@p2_val, column2),
(snip)

[Razvan Socol]

valentin tihomirov wrote:

DECLARE @val varchar(100)
SELECT @val ='vvvv' -- making val longer breaks the code
exec sp_executesql N'update tt SET @name=@val'
, N'@name varchar(4), @val varchar(100)'
, 'col1', @val

[...]

Am I missing something?

Yes. You seem to believe that the above code would update the col1
column in the tt table, setting it's value to 'vvvv'. That is not the
case. Actually, the above code puts the value 'vvvv' in the variable
@name. Obviously, that will cause an error if you try to put a value
longer than 4 characters into a varchar(4) variable.

What you probably want is this code:

DECLARE @sql nvarchar(4000), @val varchar(100), @name varchar(4)
SET @val ='vvvv'
SET @name ='col1'
SET @sql=N'update tt SET '+QUOTENAME(@name)+'=@val'
exec sp_executesql @sql, N'@val varchar(100)', @val

--
Razvan Socol
SQL Server MVP

[David Portas]

"valentin tihomirov" <V_*********@best.eewrote in message
news:eN**************@TK2MSFTNGP02.phx.gbl...

Hello,

-=PREHISTORY=-

Occasionally, I wanted to rewrite my UPDATE code to prevent SQL injection.
I generated the SQL query

UPDATE tt
SET
@p1_name = @p1_val
, @p2_name = @p2_val
, ...

along with the pairs of paramz:

new SqlParameter(formalName, actualName);
new SqlParameter(formalValue, actualValue);

for every column. Now, user can send name-value pairs the corresponding
columns will be updated safely. However, this fails if a value is longer
than 4 chars:

"System.Data.SqlClient.SqlException: String or binary data would be
truncated. The statement has been terminated."

Note, the code still works if the column names are not parametrized. As
this is weired, so I have decided to trial in pure SQL.
-=SQL=-

Here is the code to execute:

DECLARE @val varchar(100)
SELECT @val ='vvvv' -- making val longer breaks the code
exec sp_executesql N'update tt SET @name=@val'
, N'@name varchar(4), @val varchar(100)'
, 'col1', @val
Making the value one char longer will end up in the suspiciously famous
"Msg 8152, Level 16, State 14, Line 1 String or binary data would be
truncated."

THE INTERESTING thing is that the maximal allowed @val length is
determined by the @name parameter definition! Try yourslf by changing the
declaration varchar(4) -varchar(5) and this will allow entering 5-symbol
values.

Another bug observed is possibly related to the this one. The table data
is not affected even when no error is encountered. The value is taken only
when column name is not parametrized, that is, 'UPDATE tt SET col1 = @val'
is submitted.

Am I missing something?

Not a bug. The meaning of

UPDATE tt SET @name=@val

is to assign the value @val to the variable @name. It doesn't update any
column (altough it will fire update triggers on the target table). Therefore
you should expect a truncation error if @name is declared to be smaller than
the length of @val.

You cannot parameterise column names in this way. You could do:

exec sp_executesql N'update tt SET '+QUOTENAME(@name)+N'='@val

but that's not a good idea. Column names should generally be in static code.

--
David Portas

[David Portas]

"David Portas" <RE****************************@acm.orgwrote in message
news:OO**************@TK2MSFTNGP04.phx.gbl...

>
You cannot parameterise column names in this way. You could do:

exec sp_executesql N'update tt SET '+QUOTENAME(@name)+N'='@val

My apologies. Of course you cannot do that. You need to assign it to a
variable first because complex expressions aren't permitted as parameters.

--
David Portas

[bruce barker]

you can not parameterize a column name. it will be treated as a standard
sql variable. its the variable (which is only 4 chars), getting the
truncation error in the set, not the column, as no column is specified
-- bruce (sqlwork.com)

valentin tihomirov wrote:

Hello,

-=PREHISTORY=-

Occasionally, I wanted to rewrite my UPDATE code to prevent SQL injection. I
generated the SQL query

UPDATE tt
SET
@p1_name = @p1_val
, @p2_name = @p2_val
, ...

along with the pairs of paramz:

new SqlParameter(formalName, actualName);
new SqlParameter(formalValue, actualValue);

for every column. Now, user can send name-value pairs the corresponding
columns will be updated safely. However, this fails if a value is longer
than 4 chars:

"System.Data.SqlClient.SqlException: String or binary data would be
truncated. The statement has been terminated."

Note, the code still works if the column names are not parametrized. As this
is weired, so I have decided to trial in pure SQL.
-=SQL=-

Here is the code to execute:

DECLARE @val varchar(100)
SELECT @val ='vvvv' -- making val longer breaks the code
exec sp_executesql N'update tt SET @name=@val'
, N'@name varchar(4), @val varchar(100)'
, 'col1', @val
Making the value one char longer will end up in the suspiciously famous "Msg
8152, Level 16, State 14, Line 1 String or binary data would be truncated."

THE INTERESTING thing is that the maximal allowed @val length is determined
by the @name parameter definition! Try yourslf by changing the declaration
varchar(4) -varchar(5) and this will allow entering 5-symbol values.

Another bug observed is possibly related to the this one. The table data is
not affected even when no error is encountered. The value is taken only when
column name is not parametrized, that is, 'UPDATE tt SET col1 = @val' is
submitted.

Am I missing something?

[valentin tihomirov]

but that's not a good idea. Column names should generally be in static
code.

Any arguments? How do I update the only changed record fields otherwise?

[Alex Kuznetsov]

On Oct 19, 1:15 pm, "valentin tihomirov" <[e-mail]:

but that's not a good idea. Column names should generally be in static
code.

Any arguments? How do I update the only changed record fields otherwise?

UPDATE tt
SET
column1 = COALESCE(@p1_val, column1),
column2 = COALESCE(@p2_val, column2),

[valentin tihomirov]

Excuse me for ignoring you. I just wanted to ask the one who clames that
column names must always be a static code (hardcoded?). The thigs passed as
parameters are defenetely non-static, so I must know what is suggested way
to update only specific cell(s).

[valentin tihomirov]

Thank you. Could you please clarify on what is 'column1' in your example. I
need it to be an input parameter variable, so it must be prefixed by '@' in
sql.

[David Portas]

On 19 Oct, 20:05, "valentin tihomirov" <[e-mail]:

Excuse me for ignoring you. I just wanted to ask the one who clames that
column names must always be a static code (hardcoded?). The thigs passed as
parameters are defenetely non-static, so I must know what is suggested way
to update only specific cell(s).

Create separate UPDATEs for each case. You might want to combine that
with the approach that Alex suggested if you have a large number of
possible permutations. Don't forget to code for the NULL cases if you
have nullable columns.

>From another perspective, a large number of columns being updated

individually may indicate an underlying design problem. Tables are not
2D arrays (no "cells" please!) and you won't get the most out of SQL
if you try to use them that way.

--
David Portas

[Alex Kuznetsov]

On Oct 19, 2:07 pm, "valentin tihomirov" <[e-mail]:

Thank you. Could you please clarify on what is 'column1' in your example. I
need it to be an input parameter variable, so it must be prefixed by '@' in
sql.

there is no need to provide column names as parameters whatsoever.
suppose you have the following table and update procedure:
create table data.tt(tt_PK int not null,
col1 int null,
col2 int null,
col3 int null)
go
create procedure writers.update_tt
@tt_PK int,
@col1 int = null,
@col2 int = null,
@col3 int =null
as
update data.tt set
col1 = coalesce(@col1, col1),
col2 = coalesce(@col1, col2),
col3 = coalesce(@col1, col3)
where tt_pk = @tt_pk
go

to update just one column col2, run the following script

exec writers.update_tt @tt_pk = 1, @col2 = 12

[valentin tihomirov]

Now, having the update procedure, the need to parametrize names is
eliminated!

[Erland Sommarskog]

valentin tihomirov (V_*********@best.ee) writes:

Excuse me for ignoring you. I just wanted to ask the one who clames that
column names must always be a static code (hardcoded?). The thigs passed
as parameters are defenetely non-static, so I must know what is
suggested way to update only specific cell(s).

In a well-designed database, each column represents a distinct attribute,
why parameterisation does not make sense.

In less well-designed database you can try:

UPDATE tbl
SET col1 = CASE @col WHEN 'col1' THEN @value ELSE col1 END,
col1 = CASE @col WHEN 'col2' THEN @value ELSE col2 END,
...


--
Erland Sommarskog, SQL Server MVP, es****@sommarskog.se

Books Online for SQL Server 2005 at
http://www.microsoft.com/technet/pro...ads/books.mspx
Books Online for SQL Server 2000 at
http://www.microsoft.com/sql/prodinf...ons/books.mspx

[valentin tihomirov]

[i]

In a well-designed database, each column represents a distinct attribute,

This means that the attributes can be updated independently of the others.

[II]

why parameterisation does not make sense.

This contradicts to [i]

In less well-designed database you can try:

UPDATE tbl
SET col1 = CASE @col WHEN 'col1' THEN @value ELSE col1 END,
col1 = CASE @col WHEN 'col2' THEN @value ELSE col2 END,
...

This seems to be a NULL-values problem solving alternative to the COALESE.

[Erland Sommarskog]

valentin tihomirov (V_*********@best.ee) writes:

[i]

>In a well-designed database, each column represents a distinct attribute,

This means that the attributes can be updated independently of the others.

Yes. And? The point is you know which column you will update, and you
will write your update statement accordingly.

[II]

>why parameterisation does not make sense.

This contradicts to [i]

It appears to me that most time when people want to parameterise
there UPDATE statements is when they have columns like sales_jan,
sales_feb, etc.
--
Erland Sommarskog, SQL Server MVP, es****@sommarskog.se

Books Online for SQL Server 2005 at
http://www.microsoft.com/technet/pro...ads/books.mspx
Books Online for SQL Server 2000 at
http://www.microsoft.com/sql/prodinf...ons/books.mspx

[valentin tihomirov]

"Erland Sommarskog" <es****@sommarskog.sewrote in message
news:Xn**********************@127.0.0.1...

valentin tihomirov (V_*********@best.ee) writes:

>[i]

>>In a well-designed database, each column represents a distinct
attribute,

This means that the attributes can be updated independently of the
others.

Yes. And? The point is you know which column you will update, and you
will write your update statement accordingly.

The Independence of cells from each other within a record does mean that a
"well-designed" table is a 2d array of "cells". The point that it is user
who knows which of them needs an update and signals the DB accordingly.
There is no need to transfer and update the whole row (or column), when only
one cell is changed.

>

>[II]

>>why parameterisation does not make sense.

This contradicts to [i]

It appears to me that most time when people want to parameterise
there UPDATE statements is when they have columns like sales_jan,
sales_feb, etc.

Do you mean that it is the uniformity. which is the criteria, or it is a
number of columns?

>

--
Erland Sommarskog, SQL Server MVP, es****@sommarskog.se

Books Online for SQL Server 2005 at
http://www.microsoft.com/technet/pro...ads/books.mspx
Books Online for SQL Server 2000 at
http://www.microsoft.com/sql/prodinf...ons/books.mspx

[Erland Sommarskog]

valentin tihomirov (V_*********@best.ee) writes:

The Independence of cells from each other within a record does mean that
a "well-designed" table is a 2d array of "cells". The point that it is
user who knows which of them needs an update and signals the DB
accordingly. There is no need to transfer and update the whole row (or
column), when only one cell is changed.

It's not really a matrix, as there are very different behaviour for rows
and columns. Particularly, a colunm has a distinct data type, an update
statement with parameters for one column is not going to be like the
update statement for a different column.

Furthermore, the column names are given. There is a finite set of columns
in the table you can update. Whereas the key values that identify the
rows are typically drawn from a virtually unlimited set of values.
--
Erland Sommarskog, SQL Server MVP, es****@sommarskog.se

Books Online for SQL Server 2005 at
http://www.microsoft.com/technet/pro...ads/books.mspx
Books Online for SQL Server 2000 at
http://www.microsoft.com/sql/prodinf...ons/books.mspx

[Razvan Socol]

valentin tihomirov wrote:
[...]

it is user who knows which of them needs an update and signals the DB
accordingly. There is no need to transfer and update the whole row
(or column), when only one cell is changed.

I agree with you here. The way that the user would signal the DB than
only some columns need to be updated would be to write an UPDATE
statement accordingly, for example:

UPDATE tbl SET col1='x', col3=7 WHERE id=100

or (to make better use of cached plans):

EXEC sp_executesql N'UPDATE tbl SET col1=@p1, col3=@p2 WHERE id=@p3',
N'@p1 varchar(10), @p2 int, @p3 int', 'x', 7, 100

Why would you want to use a stored procedure for this ?

--
Razvan Socol
SQL Server MVP

[valentin tihomirov]

Why would you want to use a stored procedure for this ?

It is advised to enforce security (mod access to tables is forbidden) in
addition to the speed.

[Razvan Socol]

valentin tihomirov wrote:

Why would you want to use a stored procedure for this ?

It is advised to enforce security (mod access to tables is forbidden)
in addition to the speed.

In my opinion, using a stored procedure which executes dynamic SQL will
not be any better than executing the sp_executesql procedure directly,
in both security and speed. You still need to allow the user direct
access to the tables (unless you sign your procedure with a
certificate) and there will be parametrised execution plans cached for
sp_executesql anyway.

I'd appreciate if Erland and the other MVP-s could post their opinion,
too (supporting or opposing my opinion, with better arguments).

--
Razvan Socol
SQL Server MVP

[Erland Sommarskog]

Razvan Socol (rs****@gmail.com) writes:

In my opinion, using a stored procedure which executes dynamic SQL will
not be any better than executing the sp_executesql procedure directly,
in both security and speed. You still need to allow the user direct
access to the tables (unless you sign your procedure with a
certificate) and there will be parametrised execution plans cached for
sp_executesql anyway.

I'd appreciate if Erland and the other MVP-s could post their opinion,
too (supporting or opposing my opinion, with better arguments).

There is really not much add. Generally, I tend to think that for a stored
procedure with dynamic SQL to be worthwhile there need to be some level of
complexity that is worth packaging. Code like

'UPDATE tbl SET ' + quotename(@colname) + ' = @value ' +
WHERE somekey = @key'

does not really meet that level. And it looks clunky, because you will need
different procedure for different data types. Or use sql_variant.

In practice, a GUI usually goes with a procedure that updates most columns
in the table (those exposed in the GUI). If there are other function where
only a single column needs to be updated, you would typically have a
separate procedure for that.
--
Erland Sommarskog, SQL Server MVP, es****@sommarskog.se

Books Online for SQL Server 2005 at
http://www.microsoft.com/technet/pro...ads/books.mspx
Books Online for SQL Server 2000 at
http://www.microsoft.com/sql/prodinf...ons/books.mspx

[Razvan Socol]

Erland Sommarskog wrote:

In practice, a GUI usually goes with a procedure that updates most
columns in the table (those exposed in the GUI). If there are other
function where only a single column needs to be updated, you would
typically have a separate procedure for that.

And if the GUI is smart enough to know which columns have been changed
by the user (and wants to update only those columns), then it should
build a dynamic UPDATE statement on the client side and send it to
sp_executesql, along with parameters for the values of those columns.

--
Razvan Socol
SQL Server MVP

[Erland Sommarskog]

Razvan Socol (rs****@gmail.com) writes:

Erland Sommarskog wrote:

>In practice, a GUI usually goes with a procedure that updates most
columns in the table (those exposed in the GUI). If there are other
function where only a single column needs to be updated, you would
typically have a separate procedure for that.

And if the GUI is smart enough to know which columns have been changed
by the user (and wants to update only those columns), then it should
build a dynamic UPDATE statement on the client side and send it to
sp_executesql, along with parameters for the values of those columns.

And this really brings us to the point, if the GUI has all the value of
all those columns, then how much do you really gain by just updating
the columns that have actually changed? You save some network bandwidth,
but that's about it. OK, if there is a trigger on the table with
IF UPDATED in it, there may be more to win. Still it's not obvious
that this warrants the increased gain in complexity.

Having a stored procedure which handles all the columns owned by this
particular GUI appears to be a simple way to go.
--
Erland Sommarskog, SQL Server MVP, es****@sommarskog.se

Books Online for SQL Server 2005 at
http://www.microsoft.com/technet/pro...ads/books.mspx
Books Online for SQL Server 2000 at
http://www.microsoft.com/sql/prodinf...ons/books.mspx