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]

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

 

[David Portas]

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.

 

[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)

 

[David Portas]

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.

 

[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]

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]

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.

 

[Alex Kuznetsov]

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]

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, (e-mail address removed)

Books Online for SQL Server 2005 at
http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx
Books Online for SQL Server 2000 at
http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx

 

[valentin tihomirov]

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

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]

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

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, (e-mail address removed)

Books Online for SQL Server 2005 at
http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx
Books Online for SQL Server 2000 at
http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx

 

[valentin tihomirov]

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

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]

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, (e-mail address removed)

Books Online for SQL Server 2005 at
http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx
Books Online for SQL Server 2000 at
http://www.microsoft.com/sql/prodinfo/previousversions/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 ?

 

[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.