June 10, 2015 at 3:54 pm
Lynn Pettis (6/10/2015)
Jacob Wilkins (6/10/2015)
It just has the QUOTENAME and ISNULL mixed up for the database name. Flip them and it will work. As is it's trying to pass DB_NAME() as the quote character to QUOTENAME, and passing an invalid quote character just makes it output NULL.Cheers!
Actually, I think is a bit more than that. Not my code so I am not going to debug it. I'll leave that to Scott since he wrote it.
There was an additional problem, yes (if you literally just switched the position of QUOTENAME and ISNULL, it added the period at the end of the database name to be quoted, instead of between the quoted database name and quoted schema name). I was only explaining why the database name was being left off when you passed in strings like the one you mentioned 🙂
June 10, 2015 at 4:40 pm
Been busy today, but I think I've got the errors fixed now. I'm definitely not using XML and STUFF when a few simple functions will do. Just the chance of a "special" xml character is enough to nix that idea for me. Yes, yes, names "shouldn't" have those characters in them, but when you load, for example, Excel spreadsheets into tables, you get all kinds of odd characters in the name.
ALTER PROCEDURE sampleprocedure
@tablename AS varchar(4000),
@date AS date
AS
SET NOCOUNT ON;
DECLARE @query nvarchar(4000);
SET @query = 'SELECT * FROM ' +
ISNULL(QUOTENAME(PARSENAME(@tablename, 3)) + '.', '') +
ISNULL(QUOTENAME(PARSENAME(@tablename, 2)) + '.', CASE WHEN PARSENAME(@tablename, 3) IS NULL THEN '' ELSE '.' END) +
QUOTENAME(PARSENAME(@tablename, 1)) +
' WHERE invoiceDate >= ''' + CONVERT(varchar(8), @date, 112) + ''''
PRINT @query
EXEC(@query)
GO --end of proc
SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".
June 10, 2015 at 5:42 pm
srgaddam2 (6/10/2015)
Hi,I need some help in building the dynamic sql . When I create below script and executed the procedure passing the table name and date values its giving me error saying "Incorrect syntax near '>'". Any help is very highly appreciated.
Create PROCEDURE sampleprocedure
@tablename AS VARCHAR(4000),
@date as date
AS
BEGIN
declare @table varchar(1000)
declare @invoiceDate date
set @table = @tablename
set @invoiceDate = @date
declare @query nvarchar(1000);
set @query='SELECT * FROM '+ @table +'where invoiceDate >= @date';
exec(@query)
END)
Thanks...
Why are your invoices split up into separate tables without some form of partitioning in place?
--Jeff Moden
Change is inevitable... Change for the better is not.
June 10, 2015 at 7:26 pm
HI All,
Thanks a ton for the responses. I used below query by Scott. I removed the function quotename and it worked.
CREATE PROCEDURE sampleprocedure
@tablename AS varchar(4000),
@date AS date
AS
SET NOCOUNT ON;
DECLARE @query nvarchar(4000);
SET @query = 'SELECT * FROM ' + @tablename +
' WHERE invoiceDate >= ''' + CONVERT(varchar(8), @date, 112) + ''''
EXEC(@query)
GO --end of proc.
Thanks again for all responses.:-)
Heera.
June 10, 2015 at 8:08 pm
Ok... first of all, that query is susceptible to SQL Injection, which is still the leading cause of successful hack attacks. At the very least, wrap a QUOTENAME around the @tablename variable.
Second, you didn't answer my question about why you have multiple tables to store invoices in without some form of partitioning in place. You could avoid all the hoopla with Dynamic SQL with a bit of thoughtful re-engineering while making index maintenance, backups, restores, and archiving of old information a whole lot easier and faster to boot.
--Jeff Moden
Change is inevitable... Change for the better is not.
June 10, 2015 at 9:13 pm
Jeff,
I am new to SQL server, so I would appreciate if you could help me understand the SQL hacking concept. What is the purpose of quotename? How can I avoid SQL hacking when using dynamic SQL ?
And the reson for me building dynamic SQL is because I wanted to create a stored procedure and call it from QlikView in a loop by passing different tables as variables. I didnot want to bring all the invoice data to QlikView for some performance issues and hence I was using a where clause to restrict the data . In real I would just use one variable to pass table name . Hope I answer your question.
Heera
June 11, 2015 at 7:26 am
srgaddam2 (6/10/2015)
Jeff,I am new to SQL server, so I would appreciate if you could help me understand the SQL hacking concept. What is the purpose of quotename? How can I avoid SQL hacking when using dynamic SQL ?
And the reson for me building dynamic SQL is because I wanted to create a stored procedure and call it from QlikView in a loop by passing different tables as variables. I didnot want to bring all the invoice data to QlikView for some performance issues and hence I was using a where clause to restrict the data . In real I would just use one variable to pass table name . Hope I answer your question.
Heera
What Jeff is referring to is called SQL injection. If you search the web for the term, you'll find lots of information on it. Your procedure is vulnerable to it. To illustrate, what if I call your procedure with the following statement?
execute dbo.sampleprocedure @tablename = '(select ''dummy'' name) x union all select name from sys.tables; --', @date = '01/01/2015';
This will return a list of all the tables in your database, which is the first step in stealing data.
June 11, 2015 at 7:54 am
Thanks Wagner, I did some research on sql injection and I understood it.
Thanks again to everyone for responding on this post.
June 11, 2015 at 8:24 am
srgaddam2 (6/10/2015)
Jeff,I am new to SQL server, so I would appreciate if you could help me understand the SQL hacking concept. What is the purpose of quotename? How can I avoid SQL hacking when using dynamic SQL ?
And the reson for me building dynamic SQL is because I wanted to create a stored procedure and call it from QlikView in a loop by passing different tables as variables. I didnot want to bring all the invoice data to QlikView for some performance issues and hence I was using a where clause to restrict the data . In real I would just use one variable to pass table name . Hope I answer your question.
Heera
Ed Wagner hit the nail on the head and as I mentioned, it's called "SQL Injection" and that's the term that you would need to do a web search for. Just to get you started, here's one of the links. Don't let it be the only link you research.
http://en.wikipedia.org/wiki/SQL_injection
To make a much longer shorter, any form of dynamic SQL that uses strings that are given to it to concatenate together an SQL Command are a way for hackers to gain access to or destroy your database(s) and, frequently, they make their way in with "sysadmin" privs because of the mistakes that many programmers and DBAs make.
The use of QUOTENAME is just one aspect of defending against SQL Injection because it not only encapsulates the variable, but it changes certain characters in the variable to make use of the variable more (but not necessarily totally) injection proof. I'll also state that most vulnerabilities aren't in stored procedures (but you still need to think they are to be safe). Rather, they're usually in front end code that does similar concatenation in an un-parameterized fashion.
I don't have the time for a full explanation so the next thing that I suggest you research is "How to prevent SQL Injection" with the caution that any article that states that you should never use dynamic SQL is full of hooie. It's a powerful and very useful tool that can do the seemingly impossible and it should not be avoided. You just have to follow certain rules to use it without enabling SQL Injection attacks.
You should also look up sp_ExecuteSQL and learn how to use it correctly because it's the equivalent of parameterizing front end code. You should also carefully read and understand the article at the following URL.
http://sqlinthewild.co.za/index.php/2009/03/19/catch-all-queries/
Additionally, most of the methods talked about will NOT prevent SQL Injection when you want dynamic table or column names. The use of QUOTENAME helps but the contents of the variable carrying the object name should be verified against sys.tables or sys.columns to make sure that the variable does, in fact, carry a bona fide object name before being used in dynamic SQL.
Last but not least, there are powerful methods to eliminate the need to use multiple tables for things like invoice detail. There is no need to bring "all" of the data as you say your trying to avoid. There are no "performance issues" even with a single table if it's designed correctly. If you provide some more information as to how your invoice tables are designed and what the criteria is for why a given invoice would live in one table or another, we might be able to help you make the requirement for dynamic SQL simply go away.
--Jeff Moden
Change is inevitable... Change for the better is not.
June 11, 2015 at 8:43 am
Jeff Moden (6/11/2015)
srgaddam2 (6/10/2015)
Jeff,I am new to SQL server, so I would appreciate if you could help me understand the SQL hacking concept. What is the purpose of quotename? How can I avoid SQL hacking when using dynamic SQL ?
And the reson for me building dynamic SQL is because I wanted to create a stored procedure and call it from QlikView in a loop by passing different tables as variables. I didnot want to bring all the invoice data to QlikView for some performance issues and hence I was using a where clause to restrict the data . In real I would just use one variable to pass table name . Hope I answer your question.
Heera
Ed Wagner hit the nail on the head and as I mentioned, it's called "SQL Injection" and that's the term that you would need to do a web search for. Just to get you started, here's one of the links. Don't let it be the only link you research.
http://en.wikipedia.org/wiki/SQL_injection
To make a much longer shorter, any form of dynamic SQL that uses strings that are given to it to concatenate together an SQL Command are a way for hackers to gain access to or destroy your database(s) and, frequently, they make their way in with "sysadmin" privs because of the mistakes that many programmers and DBAs make.
The use of QUOTENAME is just one aspect of defending against SQL Injection because it not only encapsulates the variable, but it changes certain characters in the variable to make use of the variable more (but not necessarily totally) injection proof. I'll also state that most vulnerabilities aren't in stored procedures (but you still need to think they are to be safe). Rather, they're usually in front end code that does similar concatenation in an un-parameterized fashion.
I don't have the time for a full explanation so the next thing that I suggest you research is "How to prevent SQL Injection" with the caution that any article that states that you should never use dynamic SQL is full of hooie. It's a powerful and very useful tool that can do the seemingly impossible and it should not be avoided. You just have to follow certain rules to use it without enabling SQL Injection attacks.
You should also look up sp_ExecuteSQL and learn how to use it correctly because it's the equivalent of parameterizing front end code. You should also carefully read and understand the article at the following URL.
http://sqlinthewild.co.za/index.php/2009/03/19/catch-all-queries/
Additionally, most of the methods talked about will NOT prevent SQL Injection when you want dynamic table or column names. The use of QUOTENAME helps but the contents of the variable carrying the object name should be verified against sys.tables or sys.columns to make sure that the variable does, in fact, carry a bona fide object name before being used in dynamic SQL.
Last but not least, there are powerful methods to eliminate the need to use multiple tables for things like invoice detail. There is no need to bring "all" of the data as you say your trying to avoid. There are no "performance issues" even with a single table if it's designed correctly. If you provide some more information as to how your invoice tables are designed and what the criteria is for why a given invoice would live in one table or another, we might be able to help you make the requirement for dynamic SQL simply go away.
Jeff hit this part on the head, and it is what I kept pushing but it appears everyone else on this thread (Scott included) kept ignoring.
You should also look up sp_ExecuteSQL and learn how to use it correctly because it's the equivalent of parameterizing front end code.
June 11, 2015 at 9:03 am
I admit I can't imagine this type of proc being exposed to an external application or user. It seems much more internal to me, and thus the chance for rogue injection seems low enough to not be worth big effort to prevent it.
If for some bizarre reason you want web users to have access to a proc that lists all data from an invoice table (!!), then, yes, be very careful adding safeguards to prevent SQL injection. But, injection or no, be prepared to explain to auditors why on earth you're allowing external users to view any invoices, let alone a big batch of them.
SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".
June 11, 2015 at 9:25 am
ScottPletcher (6/11/2015)
But, injection or no, be prepared to explain to auditors why on earth you're allowing external users to view any invoices, let alone a big batch of them.
Amen to that!
--Jeff Moden
Change is inevitable... Change for the better is not.
June 11, 2015 at 9:37 am
Jeff Moden (6/11/2015)
ScottPletcher (6/11/2015)
But, injection or no, be prepared to explain to auditors why on earth you're allowing external users to view any invoices, let alone a big batch of them.Amen to that!
Yeah, that audit question is likely to cause you some significant pain.
As for a reporting system not being exposed to the internet, there's no way to know where the future development on the OP's project is going to go. The things that seem insane now, given enough time and new management, could seem completely reasonable in the future.
June 11, 2015 at 9:39 am
Lynn Pettis (6/11/2015)
Jeff hit this part on the head, and it is what I kept pushing but it appears everyone else on this thread (Scott included) kept ignoring.You should also look up sp_ExecuteSQL and learn how to use it correctly because it's the equivalent of parameterizing front end code.
You covered that in post #3 back on page 1. Of course, it's the right way, but I think the point was lost in the following posts about parsing and and qualified names.
June 11, 2015 at 9:43 am
Ed Wagner (6/11/2015)
Lynn Pettis (6/11/2015)
Jeff hit this part on the head, and it is what I kept pushing but it appears everyone else on this thread (Scott included) kept ignoring.You should also look up sp_ExecuteSQL and learn how to use it correctly because it's the equivalent of parameterizing front end code.
You covered that in post #3 back on page 1. Of course, it's the right way, but I think the point was lost in the following posts about parsing and and qualified names.
Heh... agreed. Several of us have hit such things along the way on this thread. Since the OP is new to the game and for the very reasons you've stated, I thought I'd summarize it all in one place.
--Jeff Moden
Change is inevitable... Change for the better is not.
Viewing 15 posts - 16 through 30 (of 31 total)
You must be logged in to reply to this topic. Login to reply