A Microsot Bug in Dynamic Comlumns?

  • Thread starter Thread starter valentin tihomirov
  • Start date Start date
V

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?
 
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.
Click to expand...

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)
 
valentin said:
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?
Click to expand...

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
 
valentin tihomirov said:
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?
Click to expand...

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.
 
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 said:
You cannot parameterise column names in this way. You could do:

exec sp_executesql N'update tt SET '+QUOTENAME(@name)+N'='@val
Click to expand...

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.
 
but that's not a good idea. Column names should generally be in static
code.
Click to expand...

Any arguments? How do I update the only changed record fields otherwise?
 
Any arguments? How do I update the only changed record fields otherwise?
Click to expand...

UPDATE tt
SET
column1 = COALESCE(@p1_val, column1),
column2 = COALESCE(@p2_val, column2),
 
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).
 
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.
 
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).
Click to expand...

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
Click to expand...
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.
 
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.
Click to expand...

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 said:
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).
Click to expand...

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
 
In a well-designed database, each column represents a distinct attribute,
Click to expand...

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

[II]
why parameterisation does not make sense.
Click to expand...

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,
...
Click to expand...

This seems to be a NULL-values problem solving alternative to the COALESE.
 
valentin said:
In a well-designed database, each column represents a distinct attribute,
Click to expand...

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


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.
Click to expand...

This contradicts to
Click to expand...


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
 
Erland Sommarskog said:
valentin said:
In a well-designed database, each column represents a distinct
attribute,
Click to expand...

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


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


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.
Click to expand...

This contradicts to
Click to expand...


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.
Click to expand...


Do you mean that it is the uniformity. which is the criteria, or it is a
number of columns?
 
valentin said:
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.
Click to expand...

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
 
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.
Click to expand...

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 ?
 
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.
 
Back
Top