February 24, 2014 at 11:12 am
Sean Lange (2/24/2014)
patrickmcginnis59 10839 (2/24/2014)
Sean Lange (2/24/2014)
patrickmcginnis59 10839 (2/24/2014)
GilaMonster (2/23/2014)
Jim Arko (2/23/2014)
When using dynamic T-SQL you need to concatenate the variables.
SET @sqlstring = 'UPDATE table SET column = ' + @variableSure, if you want a nice little SQL Injection vulnerability. Assuming you care an iota about security, you wouldn't concatenate variables into a string and execute it.
I don't mean to hijack but I'm curious whether this would aleviate concerns:
SET @sqlstring = 'UPDATE table SET column = ' + QUOTENAME(@variable)Is this still injectable? (I couldn't do the semicolon append hack for instance once I used QUOTENAME)
Take a look here. This is pretty much the definitive article about dynamic sql.
Well I guess if Erland doesn't know, I'm guessing nobody here would know either. Consider my question invalid!
edit: http://www.sqlskills.com/blogs/kimberly/little-bobby-tables-sql-injection-and-execute-as/ discusses quotename a bit also.
edit #2: What I like about QUOTENAME is that while it defaults to surrounding the column name with '[' and ']' (which I'm figuring means the quote char is configurable), it ALSO escapes any embedded ]'s to avoid injection.
QUOTENAME is one way to deal with injection. To me it seems like the wrong tool for the job though. It is a lot easier to parameterize queries (even better as a stored proc) than to have to wrap everything in QUOTENAME.
It looks like parameterizing is more work in this case, because the column name looks to be CALCULATED. Obviously then he has the responsibility to calculate the proper column name, and to parameterize this query he would then have to additionally introduce a conditional statement for EACH POSSIBLE CALCULATED COLUMN NAME. Now granted, thats not significantly harder (heck, I'd pull up my macro processor and be off to the races), but since he's already taking responsibility for the possible values of column name then if quotename works, then I'm wondering how quotename would be harder than parameterizing the query (unless of course, quotename doesn't really work, hence my question).
This was sort of what I was thinking, while I use sp_executesql, I still dynamically set the column name:
DECLARE @MONTHP INT
DECLARE @REPORTEDMONTHS VARCHAR(100)
DECLARE @PLUSONE INT
DECLARE @NAMECOLUMN VARCHAR(3)
DECLARE @sql NVARCHAR(2000);
DECLARE @col NVARCHAR(10);
--Cursor
DECLARE @MyCursor CURSOR
DECLARE @APP_ID VARCHAR(100)
DECLARE @ParmDefinition nvarchar(500);
SET @ParmDefinition = N'@MONTHP INT, @APP_ID VARCHAR(100)'
SET @MyCursor = CURSOR FAST_FORWARD
FOR
SELECT APP_ID, MONTHP, REPORTEDMONTHS
FROM TEMPMONTHLYMATCHES_REPORTEDMONTHS
OPEN @MyCursor
FETCH NEXT FROM @MyCursor
INTO @APP_ID,@MONTHP,@REPORTEDMONTHS
WHILE @@FETCH_STATUS = 0
BEGIN
SET @PLUSONE = @REPORTEDMONTHS+1
SET @NAMECOLUMN = 'MON'
SET @col = (SELECT CONVERT(varchar(5), @NAMECOLUMN) + CONVERT(varchar(5), @PLUSONE))
SET @sql = REPLACE(@sql, N'@col', @col);
--DYNAMIC SQL PART
set @SQL =
N'UPDATE FY08_ALL SET ' + QUOTENAME(@col) + ' = @MONTHP WHERE APP_ID = @APP_ID;'
;
--EXEC(@sql); REPLACED WITH
exec sp_executesql @SQL, @ParmDefinition, @MONTHP, @APP_ID
PRINT @APP_ID
PRINT @COL
FETCH NEXT FROM @MyCursor
INTO @APP_ID,@MONTHP,@REPORTEDMONTHS
END
CLOSE @MyCursor
DEALLOCATE @MyCursor
Obviously theres a chance I'm completely confused so no biggie!
February 24, 2014 at 12:19 pm
patrickmcginnis59 10839 (2/24/2014)
Sean Lange (2/24/2014)
patrickmcginnis59 10839 (2/24/2014)
Sean Lange (2/24/2014)
patrickmcginnis59 10839 (2/24/2014)
GilaMonster (2/23/2014)
Jim Arko (2/23/2014)
When using dynamic T-SQL you need to concatenate the variables.
SET @sqlstring = 'UPDATE table SET column = ' + @variableSure, if you want a nice little SQL Injection vulnerability. Assuming you care an iota about security, you wouldn't concatenate variables into a string and execute it.
I don't mean to hijack but I'm curious whether this would aleviate concerns:
SET @sqlstring = 'UPDATE table SET column = ' + QUOTENAME(@variable)Is this still injectable? (I couldn't do the semicolon append hack for instance once I used QUOTENAME)
Take a look here. This is pretty much the definitive article about dynamic sql.
Well I guess if Erland doesn't know, I'm guessing nobody here would know either. Consider my question invalid!
edit: http://www.sqlskills.com/blogs/kimberly/little-bobby-tables-sql-injection-and-execute-as/ discusses quotename a bit also.
edit #2: What I like about QUOTENAME is that while it defaults to surrounding the column name with '[' and ']' (which I'm figuring means the quote char is configurable), it ALSO escapes any embedded ]'s to avoid injection.
QUOTENAME is one way to deal with injection. To me it seems like the wrong tool for the job though. It is a lot easier to parameterize queries (even better as a stored proc) than to have to wrap everything in QUOTENAME.
It looks like parameterizing is more work in this case, because the column name looks to be CALCULATED. Obviously then he has the responsibility to calculate the proper column name, and to parameterize this query he would then have to additionally introduce a conditional statement for EACH POSSIBLE CALCULATED COLUMN NAME. Now granted, thats not significantly harder (heck, I'd pull up my macro processor and be off to the races), but since he's already taking responsibility for the possible values of column name then if quotename works, then I'm wondering how quotename would be harder than parameterizing the query (unless of course, quotename doesn't really work, hence my question).
This was sort of what I was thinking, while I use sp_executesql, I still dynamically set the column name:
DECLARE @MONTHP INT
DECLARE @REPORTEDMONTHS VARCHAR(100)
DECLARE @PLUSONE INT
DECLARE @NAMECOLUMN VARCHAR(3)
DECLARE @sql NVARCHAR(2000);
DECLARE @col NVARCHAR(10);
--Cursor
DECLARE @MyCursor CURSOR
DECLARE @APP_ID VARCHAR(100)
DECLARE @ParmDefinition nvarchar(500);
SET @ParmDefinition = N'@MONTHP INT, @APP_ID VARCHAR(100)'
SET @MyCursor = CURSOR FAST_FORWARD
FOR
SELECT APP_ID, MONTHP, REPORTEDMONTHS
FROM TEMPMONTHLYMATCHES_REPORTEDMONTHS
OPEN @MyCursor
FETCH NEXT FROM @MyCursor
INTO @APP_ID,@MONTHP,@REPORTEDMONTHS
WHILE @@FETCH_STATUS = 0
BEGIN
SET @PLUSONE = @REPORTEDMONTHS+1
SET @NAMECOLUMN = 'MON'
SET @col = (SELECT CONVERT(varchar(5), @NAMECOLUMN) + CONVERT(varchar(5), @PLUSONE))
SET @sql = REPLACE(@sql, N'@col', @col);
--DYNAMIC SQL PART
set @SQL =
N'UPDATE FY08_ALL SET ' + QUOTENAME(@col) + ' = @MONTHP WHERE APP_ID = @APP_ID;'
;
--EXEC(@sql); REPLACED WITH
exec sp_executesql @SQL, @ParmDefinition, @MONTHP, @APP_ID
PRINT @APP_ID
PRINT @COL
FETCH NEXT FROM @MyCursor
INTO @APP_ID,@MONTHP,@REPORTEDMONTHS
END
CLOSE @MyCursor
DEALLOCATE @MyCursor
Obviously theres a chance I'm completely confused so no biggie!
Agreed that it might be more difficult in this case. IMO that is because the underlying issue is the denormalized structure. Wrestling with Mon1 - Mon12 columns is painful.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
February 25, 2014 at 10:07 am
bob2dis (2/23/2014)
I continued with my previous method until someone could give clarity on the above. Working with the message relating to converting as a string I now have a different error. I understand I need to use a cast or convert but I can't figure out how.I changed this from the above:
set @SQL =
'UPDATE FY08_ALL SET ' + @col + ' = '+ CAST(@MONTHP as int) + ' WHERE APP_ID = '+ @APP_ID +''
I now get the error:
Msg 245, Level 16, State 1, Line 27
Conversion failed when converting the varchar value 'UPDATE FY08_ALL SET MON2 = ' to data type int.
Since @MONTHP is already an int you probably want to cast as varchar or nvarchar to concatenate with the rest of the string?
Also were you trying to put ' quotes around the value of @APP_ID? If so you need to double them in the string:
Something like this?
set @SQL =
'UPDATE FY08_ALL SET ' + @col + ' = '+ CAST(@MONTHP as nvarchar(10)) + ' WHERE APP_ID = '''+ @APP_ID +''''
Viewing 3 posts - 16 through 17 (of 17 total)
You must be logged in to reply to this topic. Login to reply